Lunderberg commented on pull request #8102:
URL: https://github.com/apache/tvm/pull/8102#issuecomment-846041537
One suggestion, I think we should need to apply the `DecorationArrayStride`
only when `interface_buffer` is false (see below). Everything else good to
me. Fantastic debugging, and thank you very much!
General notes from reading over changes, and re-reading documentation for
similar issues:
* Do issues arise if the same struct is declared both as an interface and as
a non-interface? No, since you added the `interface_block` to the cache
lookup, they would be treated as separate types.
* Are there any other uses of `DecorationBlock` or `DecorationBufferBlock`
that should be removed? No, the only other use is in `DeclareStorageType`,
which is only used when declaring I/O by push constants or uniform buffers, so
it is fine. That said, in the future we might want to rename
`DeclareStorageType` to `DeclareIOStruct`, or add the same `interface_block`
argument to it, to avoid implying that it could be used to declare a struct
that isn't used for IO.
* Are there any other decorations that should only be used conditionally?
Yes, `DecorationArrayStride`.
* DecorationArrayStride - Looks like if an array contains a structure
decorated by `Buffer` or `BufferBlock`, then the `ArrayStride` decoration must
be omitted
([ref](https://www.khronos.org/registry/spir-v/specs/unified1/SPIRV.html#_a_id_shadervalidation_a_validation_rules_for_shader_a_href_capability_capabilities_a)).
Therefore, in `GetStructArrayType`, the `DecorationArrayStride` and should
only be applied if `interface_block` is false.
* DecorationOffset - Should be used for each structure member, whether or
not it is an interface type, no issues.
* DecorationDescriptorSet, DecorationBinding - Used for buffer
inputs/outputs, no issues.
* DecorationBuiltIn - Used for accessing WorkgroupId and
LocalInvocationId, no issues.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
