OpMemberDecorate for BuiltIn and relation to OpDecorate for Block

[jenatali]

I've been looking at some SPIR-V samples from https://github.com/dneto0/spirv-samples and found some that seem to be invalid. Specifically, they declare a structure, decorate that struct's members as builtins, but don't declare that structure to be a block. For example: https://github.com/dneto0/spirv-samples/blob/main/spvasm/SpvModuleScopeVarParserTest_BuiltinPosition_BuiltIn_Position.spvasm. Also see dneto0/spirv-samples#2

According the Vulkan spec this seems to be invalid:

be declared in a block whose top-level members are the built-ins.

I couldn't find explicit rules in the core SPIR-V spec for this, however. Consensus from some open source SPIR-V folks is looking towards this being a missed validation rule.

[gfxstrand]

I think there's some arguing back and forth to be done about whether it's a Vulkan rule or a SPIR-V rule but we can do that arguing when we pull the issue inside Khronos.

[alan-baker]

This seems to run afoul of multiple Vulkan rules. IMO it is definitely for the Vulkan spec and not SPIR-V, but it seems sufficiently covered. There is an environment rule that requires non-Block-decorated structs to have locations which are incompatible with builtins.

[dneto0]

The example is indeed invalid. @alan-baker laid out the argument:

VUID-StandaloneSpirv-Location-04917

The Location decorations must be used on an OpVariable with a structure type that is not a block

"not a block" here means "not a Block-decorated struct". And that applies in this case.

(Quibble: From context, it should be understood it's talking about variables in Input or Output storage class. The context would be clear if the rule were stated in the right section of the spec, but it's free-floating in the SPIR-V validation section. I'll raise that separately.)

And there's another rule at the end of 15.1.1 "Built-in Interface Block" says

Built-ins must not have any Location or Component decorations.

That applies generally, whether a built-in is in a block or not.

3. When validation location overlaps, the location on a variable containing a structure propagates down to its members. And so that generates a conflict as pointed out in rule 2.

So together, the example is invalid. I may have generated created the test case when the SPIRV-Tools validator wasn't validation locations, or there may be a gap in validation. I'll investigate that.

So there's no spec problem here.

[jenatali]

I may have generated created the test case when the SPIRV-Tools validator wasn't validation locations, or there may be a gap in validation.

Indeed, the SPIRV-Tools validator doesn't complain on these modules (at least the version I had built locally from earlier this year).

[dneto0]

(Quibble: From context, it should be understood it's talking about variables in Input or Output storage class. The context would be clear if the rule were stated in the right section of the spec, but it's free-floating in the SPIR-V validation section. I'll raise that separately.)
ay have generated created the test case when the SPIRV-Tools validator wasn't validation locations, or there may be a gap in validation. I'll investigate that.

I updated an Khronons-internal Vulkan documentation issue #2737 to clarify these VUIDs are about Input/Output variables.

[dneto0]

I may have generated created the test case when the SPIRV-Tools validator wasn't validation locations, or there may be a gap in validation.

Indeed, the SPIRV-Tools validator doesn't complain on these modules (at least the version I had built locally from earlier this year).

+1

It turns out the validation support for these rules has been proposed, but are held up in review. KhronosGroup/SPIRV-Tools#4608

[dneto0]

I have a patch to the validator to check the necessary rule: KhronosGroup/SPIRV-Tools#4665 (The older 4608 missed the mark).

[raunraun]

anything left to address here?

[raunraun]

closing - please reopen if needed.