DXIL: Should use ComponentType rather than ElementType for enumeration
- Dominant language
- LLVM
- Stars
- 40.5k
- Forks
- 18.7k
- PR merge metrics
- PR metrics pending
Description
https://github.com/llvm/llvm-project/blob/66da12ae69757690dff48e4c43816c753797babd/llvm/include/llvm/Support/DXILABI.h#L59-L80
I think the name `ElementType` used in llvm-project here was not the right name to use, and the comment is overly narrow in stating the purpose of this enumeration. The purpose is a standard enumeration that can capture a scalar component type (and interpretation) in DXIL that originates from HLSL.
`ComponentType` is used in multiple places to indicate a single scalar component type, while "Element" terminology is used in various contexts: buffers, signatures, arrays, and possibly more, where element types are not necessary scalar.
I believe the source of the confusion is the naming of one (or two) DXIL metadata index constants in `DxilMetadataHelper.h` in DXC.
One is [`kDxilTypedBufferElementTypeTag`](https://github.com/microsoft/DirectXShaderCompiler/blob/201a5b598caed7158c0ea2aadf1a6e1590afa218/include/dxc/DXIL/DxilMetadataHelper.h#L232), identifying the extended tag ID for capturing the HLSL `ComponentType` for a typed resource element. Since the number of components for a typed resource is defined by the DXGI type in the runtime descriptor, the only HLSL property of the element type that needs to be captured to DXIL is the `ComponentType`. The equivalent Tag id in this llvm-project is:
https://github.com/llvm/llvm-project/blob/66da12ae69757690dff48e4c43816c753797babd/llvm/include/llvm/Support/DXILABI.h#L82-L84
The second is [`kDxilSignatureElementType`](https://github.com/microsoft/DirectXShaderCompiler/blob/201a5b598caed7158c0ea2aadf1a6e1590afa218/include/dxc/DXIL/DxilMetadataHelper.h#L155), identifying the position index of the element's `ComponentType` used in the DXIL signature element metadata list. The overall signature element is more than just this component type (it has `Rows` and `Cols`), but since the component type must be homogenous for the element, one component type is all that's needed to identify this aspect of the element type from HLSL. This one has no equivalent in llvm-project, and it may never have, as it will likely just rely on a hard-coded ordering in metadata serialization code.
It would have been better to rename the metadata id than to change the name of the `ComponentType` enumeration to `ElementType`. The former only impacts a couple points in code where this metadata is serialized, while the latter has a much broader code impact, and even impacts the language used to talk and reason about details from HLSL to DXIL IR.
I Recommend renaming this metadata id for clarity: `ExtPropTags::ElementType` -> `ResourceExtPropTag::TypedElementComponentType` - for the `ComponentType` of the element of a typed resource. The rename of the enum `ExtPropTags` to `ResourceExtPropTag` is recommended because extended property tags are local to a particular context - this one only lists resource metadata extended property tags, and `Tags` -> `Tag` because the enumeration name does not normally use the plural.
Contributor guide
Research direction
Start with the ComponentType/ElementType definitions and comments in llvm/include/llvm/Support/DXILABI.h, then inspect the related metadata naming in DXC's include/dxc/DXIL/DxilMetadataHelper.h. Search for references to ExtPropTags::ElementType and the enumeration names, and consider the work complete when the recommended names are applied consistently without changing their intended metadata meaning.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- cpp
- Domain
- compilers
- Issue type
- Refactor
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 35/100