Skip to content

[DirectX] add enum for PSV resource type/kind/flag. #106227

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Aug 29, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 23 additions & 2 deletions llvm/include/llvm/BinaryFormat/DXContainer.h
Original file line number Diff line number Diff line change
Expand Up @@ -299,6 +299,27 @@ enum class InterpolationMode : uint8_t {

ArrayRef<EnumEntry<InterpolationMode>> getInterpolationModes();

#define RESOURCE_TYPE(Val, Enum) Enum = Val,
enum class ResourceType : uint32_t {
#include "DXContainerConstants.def"
};

ArrayRef<EnumEntry<ResourceType>> getResourceTypes();

#define RESOURCE_KIND(Val, Enum) Enum = Val,
enum class ResourceKind : uint32_t {
#include "DXContainerConstants.def"
};

ArrayRef<EnumEntry<ResourceKind>> getResourceKinds();

#define RESOURCE_FLAG(Val, Enum) Enum = Val,
enum class ResourceFlag : uint32_t {
#include "DXContainerConstants.def"
};

ArrayRef<EnumEntry<ResourceFlag>> getResourceFlags();

namespace v0 {
struct RuntimeInfo {
PipelinePSVInfo StageInfo;
Expand All @@ -315,7 +336,7 @@ struct RuntimeInfo {
};

struct ResourceBindInfo {
uint32_t Type;
ResourceType Type;
uint32_t Space;
uint32_t LowerBound;
uint32_t UpperBound;
Expand Down Expand Up @@ -417,7 +438,7 @@ struct RuntimeInfo : public v1::RuntimeInfo {
};

struct ResourceBindInfo : public v0::ResourceBindInfo {
uint32_t Kind;
ResourceKind Kind;
uint32_t Flags;

void swapBytes() {
Expand Down
46 changes: 46 additions & 0 deletions llvm/include/llvm/BinaryFormat/DXContainerConstants.def
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,52 @@ INTERPOLATION_MODE(8, Invalid)
#undef INTERPOLATION_MODE
#endif // INTERPOLATION_MODE

#ifdef RESOURCE_TYPE
RESOURCE_TYPE(0, Invalid)
RESOURCE_TYPE(1, Sampler)
RESOURCE_TYPE(2, CBV)
RESOURCE_TYPE(3, SRVTyped)
RESOURCE_TYPE(4, SRVRaw)
RESOURCE_TYPE(5, SRVStructured)
RESOURCE_TYPE(6, UAVTyped)
RESOURCE_TYPE(7, UAVRaw)
RESOURCE_TYPE(8, UAVStructured)
RESOURCE_TYPE(9, UAVStructuredWithCounter)

#undef RESOURCE_TYPE
#endif // RESOURCE_TYPE

#ifdef RESOURCE_KIND
RESOURCE_KIND(0, Invalid)
RESOURCE_KIND(1, Texture1D)
RESOURCE_KIND(2, Texture2D)
RESOURCE_KIND(3, Texture2DMS)
RESOURCE_KIND(4, Texture3D)
RESOURCE_KIND(5, TextureCube)
RESOURCE_KIND(6, Texture1DArray)
RESOURCE_KIND(7, Texture2DArray)
RESOURCE_KIND(8, Texture2DMSArray)
RESOURCE_KIND(9, TextureCubeArray)
RESOURCE_KIND(10, TypedBuffer)
RESOURCE_KIND(11, RawBuffer)
RESOURCE_KIND(12, StructuredBuffer)
RESOURCE_KIND(13, CBuffer)
RESOURCE_KIND(14, Sampler)
RESOURCE_KIND(15, TBuffer)
RESOURCE_KIND(16, RTAccelerationStructure)
RESOURCE_KIND(17, FeedbackTexture2D)
RESOURCE_KIND(18, FeedbackTexture2DArray)

#undef RESOURCE_KIND
#endif // RESOURCE_KIND

#ifdef RESOURCE_FLAG
RESOURCE_FLAG(0, None)
RESOURCE_FLAG(1, UsedByAtomic64)

#undef RESOURCE_FLAG
#endif // RESOURCE_FLAG

#ifdef D3D_SYSTEM_VALUE

D3D_SYSTEM_VALUE(0, Undefined)
Expand Down
3 changes: 3 additions & 0 deletions llvm/include/llvm/ObjectYAML/DXContainerYAML.h
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,9 @@ LLVM_YAML_IS_SEQUENCE_VECTOR(llvm::DXContainerYAML::SignatureParameter)
LLVM_YAML_DECLARE_ENUM_TRAITS(llvm::dxbc::PSV::SemanticKind)
LLVM_YAML_DECLARE_ENUM_TRAITS(llvm::dxbc::PSV::ComponentType)
LLVM_YAML_DECLARE_ENUM_TRAITS(llvm::dxbc::PSV::InterpolationMode)
LLVM_YAML_DECLARE_ENUM_TRAITS(llvm::dxbc::PSV::ResourceType)
LLVM_YAML_DECLARE_ENUM_TRAITS(llvm::dxbc::PSV::ResourceKind)
LLVM_YAML_DECLARE_ENUM_TRAITS(llvm::dxbc::PSV::ResourceFlag)
LLVM_YAML_DECLARE_ENUM_TRAITS(llvm::dxbc::D3DSystemValue)
LLVM_YAML_DECLARE_ENUM_TRAITS(llvm::dxbc::SigComponentType)
LLVM_YAML_DECLARE_ENUM_TRAITS(llvm::dxbc::SigMinPrecision)
Expand Down
30 changes: 30 additions & 0 deletions llvm/lib/BinaryFormat/DXContainer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -89,3 +89,33 @@ static const EnumEntry<PSV::InterpolationMode> InterpolationModeNames[] = {
ArrayRef<EnumEntry<PSV::InterpolationMode>> PSV::getInterpolationModes() {
return ArrayRef(InterpolationModeNames);
}

#define RESOURCE_TYPE(Val, Enum) {#Enum, PSV::ResourceType::Enum},

static const EnumEntry<PSV::ResourceType> ResourceTypeNames[] = {
#include "llvm/BinaryFormat/DXContainerConstants.def"
};
Comment on lines +93 to +97
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: Style-wise, we usually put the #define immediately before #include of a def file, like so:

Suggested change
#define RESOURCE_TYPE(Val, Enum) {#Enum, PSV::ResourceType::Enum},
static const EnumEntry<PSV::ResourceType> ResourceTypeNames[] = {
#include "llvm/BinaryFormat/DXContainerConstants.def"
};
static const EnumEntry<PSV::ResourceType> ResourceTypeNames[] = {
#define RESOURCE_TYPE(Val, Enum) {#Enum, PSV::ResourceType::Enum},
#include "llvm/BinaryFormat/DXContainerConstants.def"
};

I find this makes it more obvious that the two things are connected. This suggestion also applies to the enum definitions in the header.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was about to give similar feedback in that llvm/include/llvm/BinaryFormat/DXContainer.h but it pretty consistently puts the #define before the enum.

I wonder if a fix here, or maybe in a follow up, to update DXContainer to this way would be worth doing?


ArrayRef<EnumEntry<PSV::ResourceType>> PSV::getResourceTypes() {
return ArrayRef(ResourceTypeNames);
}

#define RESOURCE_KIND(Val, Enum) {#Enum, PSV::ResourceKind::Enum},

static const EnumEntry<PSV::ResourceKind> ResourceKindNames[] = {
#include "llvm/BinaryFormat/DXContainerConstants.def"
};

ArrayRef<EnumEntry<PSV::ResourceKind>> PSV::getResourceKinds() {
return ArrayRef(ResourceKindNames);
}

#define RESOURCE_FLAG(Val, Enum) {#Enum, PSV::ResourceFlag::Enum},

static const EnumEntry<PSV::ResourceFlag> ResourceFlagNames[] = {
#include "llvm/BinaryFormat/DXContainerConstants.def"
};

ArrayRef<EnumEntry<PSV::ResourceFlag>> PSV::getResourceFlags() {
return ArrayRef(ResourceFlagNames);
}
18 changes: 18 additions & 0 deletions llvm/lib/ObjectYAML/DXContainerYAML.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -254,6 +254,24 @@ void ScalarEnumerationTraits<dxbc::PSV::InterpolationMode>::enumeration(
IO.enumCase(Value, E.Name.str().c_str(), E.Value);
}

void ScalarEnumerationTraits<dxbc::PSV::ResourceType>::enumeration(
IO &IO, dxbc::PSV::ResourceType &Value) {
for (const auto &E : dxbc::PSV::getResourceTypes())
IO.enumCase(Value, E.Name.str().c_str(), E.Value);
}

void ScalarEnumerationTraits<dxbc::PSV::ResourceKind>::enumeration(
IO &IO, dxbc::PSV::ResourceKind &Value) {
for (const auto &E : dxbc::PSV::getResourceKinds())
IO.enumCase(Value, E.Name.str().c_str(), E.Value);
}

void ScalarEnumerationTraits<dxbc::PSV::ResourceFlag>::enumeration(
IO &IO, dxbc::PSV::ResourceFlag &Value) {
for (const auto &E : dxbc::PSV::getResourceFlags())
IO.enumCase(Value, E.Name.str().c_str(), E.Value);
}

void ScalarEnumerationTraits<dxbc::D3DSystemValue>::enumeration(
IO &IO, dxbc::D3DSystemValue &Value) {
for (const auto &E : dxbc::getD3DSystemValues())
Expand Down
4 changes: 2 additions & 2 deletions llvm/test/ObjectYAML/DXContainer/DomainMaskVectors.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -70,11 +70,11 @@ Parts:
NumThreadsZ: 0
ResourceStride: 24
Resources:
- Type: 2
- Type: CBV
Space: 0
LowerBound: 0
UpperBound: 0
Kind: 13
Kind: CBuffer
Flags: 0
SigInputElements:
- Name: AAA_HSFoo
Expand Down
8 changes: 4 additions & 4 deletions llvm/test/ObjectYAML/DXContainer/PSVv0-amplification.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,11 @@ Parts:
MaximumWaveLaneCount: 4294967295
ResourceStride: 16
Resources:
- Type: 1
- Type: Sampler
Space: 2
LowerBound: 3
UpperBound: 4
- Type: 128
- Type: Invalid
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry if this is a dumb question, but I'm not very familiar with the YAML printer. Is this printing the enum value of 0 (ie, "Invalid") or is the value in memory 128 and anything outside of the enum range prints "Invalid"? If it's 0, what part of this change caused the value to change?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My understanding is that for enum class types, you cannot say 128 anymore.
Have to choose from the enum names.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm just confused because I see some new definitions and I see test changes, but I don't see the producer of the value changing anywhere so it isn't clear to me what the actual difference in behaviour is. I'm guessing the new ScalarEnumerationTraits specializations are the cause, but is the value different in the actual DXContainer or is it just a display thing with the YAML?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The value should not change, unless it does not have a mapping to the enum values.
It just YAML now require the names instead of integer.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems kind of unfortunate to not see the actual value that's in the structure in our debug tools - couldn't this mean that we end up putting garbage values in the actual container without noticing it in our tests? I guess if that's how the yaml printing works it's a wider issue and doesn't need to block this change in particular.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. 128 is garbage value which should not happen.

Space: 32768
LowerBound: 8388608
UpperBound: 2147483648
Expand All @@ -48,11 +48,11 @@ Parts:
# CHECK-NEXT: MaximumWaveLaneCount: 4294967295
# CHECK-NEXT: ResourceStride: 16
# CHECK-NEXT: Resources:
# CHECK-NEXT: - Type: 1
# CHECK-NEXT: - Type: Sampler
# CHECK-NEXT: Space: 2
# CHECK-NEXT: LowerBound: 3
# CHECK-NEXT: UpperBound: 4
# CHECK-NEXT: - Type: 128
# CHECK-NEXT: - Type: Invalid
# CHECK-NEXT: Space: 32768
# CHECK-NEXT: LowerBound: 8388608
# CHECK-NEXT: UpperBound: 2147483648
Expand Down
8 changes: 4 additions & 4 deletions llvm/test/ObjectYAML/DXContainer/PSVv0-compute.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,11 @@ Parts:
MaximumWaveLaneCount: 4294967295
ResourceStride: 16
Resources:
- Type: 1
- Type: Sampler
Space: 2
LowerBound: 3
UpperBound: 4
- Type: 128
- Type: Invalid
Space: 32768
LowerBound: 8388608
UpperBound: 2147483648
Expand All @@ -46,11 +46,11 @@ Parts:
# CHECK-NEXT: MaximumWaveLaneCount: 4294967295
# CHECK-NEXT: ResourceStride: 16
# CHECK-NEXT: Resources:
# CHECK-NEXT: - Type: 1
# CHECK-NEXT: - Type: Sampler
# CHECK-NEXT: Space: 2
# CHECK-NEXT: LowerBound: 3
# CHECK-NEXT: UpperBound: 4
# CHECK-NEXT: - Type: 128
# CHECK-NEXT: - Type: Invalid
# CHECK-NEXT: Space: 32768
# CHECK-NEXT: LowerBound: 8388608
# CHECK-NEXT: UpperBound: 2147483648
Expand Down
8 changes: 4 additions & 4 deletions llvm/test/ObjectYAML/DXContainer/PSVv0-domain.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,11 @@ Parts:
MaximumWaveLaneCount: 4294967295
ResourceStride: 16
Resources:
- Type: 1
- Type: Sampler
Space: 2
LowerBound: 3
UpperBound: 4
- Type: 128
- Type: Invalid
Space: 32768
LowerBound: 8388608
UpperBound: 2147483648
Expand All @@ -52,11 +52,11 @@ Parts:
# CHECK-NEXT: MaximumWaveLaneCount: 4294967295
# CHECK-NEXT: ResourceStride: 16
# CHECK-NEXT: Resources:
# CHECK-NEXT: - Type: 1
# CHECK-NEXT: - Type: Sampler
# CHECK-NEXT: Space: 2
# CHECK-NEXT: LowerBound: 3
# CHECK-NEXT: UpperBound: 4
# CHECK-NEXT: - Type: 128
# CHECK-NEXT: - Type: Invalid
# CHECK-NEXT: Space: 32768
# CHECK-NEXT: LowerBound: 8388608
# CHECK-NEXT: UpperBound: 2147483648
Expand Down
8 changes: 4 additions & 4 deletions llvm/test/ObjectYAML/DXContainer/PSVv0-geometry.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,11 @@ Parts:
MaximumWaveLaneCount: 4294967295
ResourceStride: 16
Resources:
- Type: 1
- Type: Sampler
Space: 2
LowerBound: 3
UpperBound: 4
- Type: 128
- Type: Invalid
Space: 32768
LowerBound: 8388608
UpperBound: 2147483648
Expand Down Expand Up @@ -54,11 +54,11 @@ Parts:
# CHECK-NEXT: MaximumWaveLaneCount: 4294967295
# CHECK-NEXT: ResourceStride: 16
# CHECK-NEXT: Resources:
# CHECK-NEXT: - Type: 1
# CHECK-NEXT: - Type: Sampler
# CHECK-NEXT: Space: 2
# CHECK-NEXT: LowerBound: 3
# CHECK-NEXT: UpperBound: 4
# CHECK-NEXT: - Type: 128
# CHECK-NEXT: - Type: Invalid
# CHECK-NEXT: Space: 32768
# CHECK-NEXT: LowerBound: 8388608
# CHECK-NEXT: UpperBound: 2147483648
Expand Down
8 changes: 4 additions & 4 deletions llvm/test/ObjectYAML/DXContainer/PSVv0-hull.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,11 @@ Parts:
MaximumWaveLaneCount: 4294967295
ResourceStride: 16
Resources:
- Type: 1
- Type: Sampler
Space: 2
LowerBound: 3
UpperBound: 4
- Type: 128
- Type: Invalid
Space: 32768
LowerBound: 8388608
UpperBound: 2147483648
Expand Down Expand Up @@ -54,11 +54,11 @@ Parts:
# CHECK-NEXT: MaximumWaveLaneCount: 4294967295
# CHECK-NEXT: ResourceStride: 16
# CHECK-NEXT: Resources:
# CHECK-NEXT: - Type: 1
# CHECK-NEXT: - Type: Sampler
# CHECK-NEXT: Space: 2
# CHECK-NEXT: LowerBound: 3
# CHECK-NEXT: UpperBound: 4
# CHECK-NEXT: - Type: 128
# CHECK-NEXT: - Type: Invalid
# CHECK-NEXT: Space: 32768
# CHECK-NEXT: LowerBound: 8388608
# CHECK-NEXT: UpperBound: 2147483648
Expand Down
8 changes: 4 additions & 4 deletions llvm/test/ObjectYAML/DXContainer/PSVv0-mesh.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -23,11 +23,11 @@ Parts:
MaximumWaveLaneCount: 4294967295
ResourceStride: 16
Resources:
- Type: 1
- Type: Sampler
Space: 2
LowerBound: 3
UpperBound: 4
- Type: 128
- Type: Invalid
Space: 32768
LowerBound: 8388608
UpperBound: 2147483648
Expand Down Expand Up @@ -56,11 +56,11 @@ Parts:
# CHECK-NEXT: MaximumWaveLaneCount: 4294967295
# CHECK-NEXT: ResourceStride: 16
# CHECK-NEXT: Resources:
# CHECK-NEXT: - Type: 1
# CHECK-NEXT: - Type: Sampler
# CHECK-NEXT: Space: 2
# CHECK-NEXT: LowerBound: 3
# CHECK-NEXT: UpperBound: 4
# CHECK-NEXT: - Type: 128
# CHECK-NEXT: - Type: Invalid
# CHECK-NEXT: Space: 32768
# CHECK-NEXT: LowerBound: 8388608
# CHECK-NEXT: UpperBound: 2147483648
Expand Down
8 changes: 4 additions & 4 deletions llvm/test/ObjectYAML/DXContainer/PSVv0-pixel.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,11 @@ Parts:
MaximumWaveLaneCount: 4294967295
ResourceStride: 16
Resources:
- Type: 1
- Type: Sampler
Space: 2
LowerBound: 3
UpperBound: 4
- Type: 128
- Type: Invalid
Space: 32768
LowerBound: 8388608
UpperBound: 2147483648
Expand All @@ -50,11 +50,11 @@ Parts:
# CHECK-NEXT: MaximumWaveLaneCount: 4294967295
# CHECK-NEXT: ResourceStride: 16
# CHECK-NEXT: Resources:
# CHECK-NEXT: - Type: 1
# CHECK-NEXT: - Type: Sampler
# CHECK-NEXT: Space: 2
# CHECK-NEXT: LowerBound: 3
# CHECK-NEXT: UpperBound: 4
# CHECK-NEXT: - Type: 128
# CHECK-NEXT: - Type: Invalid
# CHECK-NEXT: Space: 32768
# CHECK-NEXT: LowerBound: 8388608
# CHECK-NEXT: UpperBound: 2147483648
Expand Down
Loading
Loading