Skip to content
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

[DirectX][NFC] Rename ShaderFlag to SHADER_FEATURE_FLAG. #82700

Merged
merged 6 commits into from
Feb 28, 2024

Conversation

python3kgae
Copy link
Contributor

This is preparation for add ShaderFlag in DXIL.

For #57925

This is prepare for add ShaderFlag in DXIL.

For llvm#57925
@llvmbot
Copy link
Collaborator

llvmbot commented Feb 22, 2024

@llvm/pr-subscribers-llvm-binary-utilities
@llvm/pr-subscribers-objectyaml

@llvm/pr-subscribers-backend-directx

Author: Xiang Li (python3kgae)

Changes

This is preparation for add ShaderFlag in DXIL.

For #57925


Full diff: https://github.com/llvm/llvm-project/pull/82700.diff

6 Files Affected:

  • (modified) llvm/include/llvm/BinaryFormat/DXContainer.h (+1-1)
  • (modified) llvm/include/llvm/BinaryFormat/DXContainerConstants.def (+37-37)
  • (modified) llvm/include/llvm/ObjectYAML/DXContainerYAML.h (+1-1)
  • (modified) llvm/lib/ObjectYAML/DXContainerYAML.cpp (+3-3)
  • (modified) llvm/lib/Target/DirectX/DXILShaderFlags.cpp (+1-1)
  • (modified) llvm/lib/Target/DirectX/DXILShaderFlags.h (+3-3)
diff --git a/llvm/include/llvm/BinaryFormat/DXContainer.h b/llvm/include/llvm/BinaryFormat/DXContainer.h
index c3dcd568216b71..fa786af0d9cda1 100644
--- a/llvm/include/llvm/BinaryFormat/DXContainer.h
+++ b/llvm/include/llvm/BinaryFormat/DXContainer.h
@@ -141,7 +141,7 @@ enum class PartType {
 #include "DXContainerConstants.def"
 };
 
-#define SHADER_FLAG(Num, Val, Str) Val = 1ull << Num,
+#define SHADER_FEATURE_INFO(Num, Val, Str) Val = 1ull << Num,
 enum class FeatureFlags : uint64_t {
 #include "DXContainerConstants.def"
 };
diff --git a/llvm/include/llvm/BinaryFormat/DXContainerConstants.def b/llvm/include/llvm/BinaryFormat/DXContainerConstants.def
index 87dd0a5cb6ba70..00caefcfd9d03f 100644
--- a/llvm/include/llvm/BinaryFormat/DXContainerConstants.def
+++ b/llvm/include/llvm/BinaryFormat/DXContainerConstants.def
@@ -11,43 +11,43 @@ CONTAINER_PART(PSG1)
 #undef CONTAINER_PART
 #endif 
 
-#ifdef SHADER_FLAG
-
-SHADER_FLAG(0, Doubles, "Double-precision floating point")
-SHADER_FLAG(1, ComputeShadersPlusRawAndStructuredBuffers, "Raw and Structured buffers")
-SHADER_FLAG(2, UAVsAtEveryStage, "UAVs at every shader stage")
-SHADER_FLAG(3, Max64UAVs, "64 UAV slots")
-SHADER_FLAG(4, MinimumPrecision, "Minimum-precision data types")
-SHADER_FLAG(5, DX11_1_DoubleExtensions, "Double-precision extensions for 11.1")
-SHADER_FLAG(6, DX11_1_ShaderExtensions, "Shader extensions for 11.1")
-SHADER_FLAG(7, LEVEL9ComparisonFiltering, "Comparison filtering for feature level 9")
-SHADER_FLAG(8, TiledResources, "Tiled resources")
-SHADER_FLAG(9, StencilRef, "PS Output Stencil Ref")
-SHADER_FLAG(10, InnerCoverage, "PS Inner Coverage")
-SHADER_FLAG(11, TypedUAVLoadAdditionalFormats, "Typed UAV Load Additional Formats")
-SHADER_FLAG(12, ROVs, "Raster Ordered UAVs")
-SHADER_FLAG(13, ViewportAndRTArrayIndexFromAnyShaderFeedingRasterizer, "SV_RenderTargetArrayIndex or SV_ViewportArrayIndex from any shader feeding rasterizer")
-SHADER_FLAG(14, WaveOps, "Wave level operations")
-SHADER_FLAG(15, Int64Ops, "64-Bit integer")
-SHADER_FLAG(16, ViewID, "View Instancing")
-SHADER_FLAG(17, Barycentrics, "Barycentrics")
-SHADER_FLAG(18, NativeLowPrecision, "Use native low precision")
-SHADER_FLAG(19, ShadingRate, "Shading Rate")
-SHADER_FLAG(20, Raytracing_Tier_1_1, "Raytracing tier 1.1 features")
-SHADER_FLAG(21, SamplerFeedback, "Sampler feedback")
-SHADER_FLAG(22, AtomicInt64OnTypedResource, "64-bit Atomics on Typed Resources")
-SHADER_FLAG(23, AtomicInt64OnGroupShared, "64-bit Atomics on Group Shared")
-SHADER_FLAG(24, DerivativesInMeshAndAmpShaders, "Derivatives in mesh and amplification shaders")
-SHADER_FLAG(25, ResourceDescriptorHeapIndexing, "Resource descriptor heap indexing")
-SHADER_FLAG(26, SamplerDescriptorHeapIndexing, "Sampler descriptor heap indexing")
-SHADER_FLAG(27, RESERVED, "<RESERVED>")
-SHADER_FLAG(28, AtomicInt64OnHeapResource, "64-bit Atomics on Heap Resources")
-SHADER_FLAG(29, AdvancedTextureOps, "Advanced Texture Ops")
-SHADER_FLAG(30, WriteableMSAATextures, "Writeable MSAA Textures")
-
-SHADER_FLAG(31, NextUnusedBit, "Next reserved shader flag bit (not a flag)")
-
-#undef SHADER_FLAG
+#ifdef SHADER_FEATURE_INFO
+
+SHADER_FEATURE_INFO(0, Doubles, "Double-precision floating point")
+SHADER_FEATURE_INFO(1, ComputeShadersPlusRawAndStructuredBuffers, "Raw and Structured buffers")
+SHADER_FEATURE_INFO(2, UAVsAtEveryStage, "UAVs at every shader stage")
+SHADER_FEATURE_INFO(3, Max64UAVs, "64 UAV slots")
+SHADER_FEATURE_INFO(4, MinimumPrecision, "Minimum-precision data types")
+SHADER_FEATURE_INFO(5, DX11_1_DoubleExtensions, "Double-precision extensions for 11.1")
+SHADER_FEATURE_INFO(6, DX11_1_ShaderExtensions, "Shader extensions for 11.1")
+SHADER_FEATURE_INFO(7, LEVEL9ComparisonFiltering, "Comparison filtering for feature level 9")
+SHADER_FEATURE_INFO(8, TiledResources, "Tiled resources")
+SHADER_FEATURE_INFO(9, StencilRef, "PS Output Stencil Ref")
+SHADER_FEATURE_INFO(10, InnerCoverage, "PS Inner Coverage")
+SHADER_FEATURE_INFO(11, TypedUAVLoadAdditionalFormats, "Typed UAV Load Additional Formats")
+SHADER_FEATURE_INFO(12, ROVs, "Raster Ordered UAVs")
+SHADER_FEATURE_INFO(13, ViewportAndRTArrayIndexFromAnyShaderFeedingRasterizer, "SV_RenderTargetArrayIndex or SV_ViewportArrayIndex from any shader feeding rasterizer")
+SHADER_FEATURE_INFO(14, WaveOps, "Wave level operations")
+SHADER_FEATURE_INFO(15, Int64Ops, "64-Bit integer")
+SHADER_FEATURE_INFO(16, ViewID, "View Instancing")
+SHADER_FEATURE_INFO(17, Barycentrics, "Barycentrics")
+SHADER_FEATURE_INFO(18, NativeLowPrecision, "Use native low precision")
+SHADER_FEATURE_INFO(19, ShadingRate, "Shading Rate")
+SHADER_FEATURE_INFO(20, Raytracing_Tier_1_1, "Raytracing tier 1.1 features")
+SHADER_FEATURE_INFO(21, SamplerFeedback, "Sampler feedback")
+SHADER_FEATURE_INFO(22, AtomicInt64OnTypedResource, "64-bit Atomics on Typed Resources")
+SHADER_FEATURE_INFO(23, AtomicInt64OnGroupShared, "64-bit Atomics on Group Shared")
+SHADER_FEATURE_INFO(24, DerivativesInMeshAndAmpShaders, "Derivatives in mesh and amplification shaders")
+SHADER_FEATURE_INFO(25, ResourceDescriptorHeapIndexing, "Resource descriptor heap indexing")
+SHADER_FEATURE_INFO(26, SamplerDescriptorHeapIndexing, "Sampler descriptor heap indexing")
+SHADER_FEATURE_INFO(27, RESERVED, "<RESERVED>")
+SHADER_FEATURE_INFO(28, AtomicInt64OnHeapResource, "64-bit Atomics on Heap Resources")
+SHADER_FEATURE_INFO(29, AdvancedTextureOps, "Advanced Texture Ops")
+SHADER_FEATURE_INFO(30, WriteableMSAATextures, "Writeable MSAA Textures")
+
+SHADER_FEATURE_INFO(31, NextUnusedBit, "Next reserved shader flag bit (not a flag)")
+
+#undef SHADER_FEATURE_INFO
 #endif
 
 #ifdef SEMANTIC_KIND
diff --git a/llvm/include/llvm/ObjectYAML/DXContainerYAML.h b/llvm/include/llvm/ObjectYAML/DXContainerYAML.h
index 66a6ac70bbea10..1ad30df36cdbf2 100644
--- a/llvm/include/llvm/ObjectYAML/DXContainerYAML.h
+++ b/llvm/include/llvm/ObjectYAML/DXContainerYAML.h
@@ -56,7 +56,7 @@ struct DXILProgram {
   std::optional<std::vector<llvm::yaml::Hex8>> DXIL;
 };
 
-#define SHADER_FLAG(Num, Val, Str) bool Val = false;
+#define SHADER_FEATURE_INFO(Num, Val, Str) bool Val = false;
 struct ShaderFlags {
   ShaderFlags() = default;
   ShaderFlags(uint64_t FlagData);
diff --git a/llvm/lib/ObjectYAML/DXContainerYAML.cpp b/llvm/lib/ObjectYAML/DXContainerYAML.cpp
index 1f03f2c7d39966..1dadcb987f215b 100644
--- a/llvm/lib/ObjectYAML/DXContainerYAML.cpp
+++ b/llvm/lib/ObjectYAML/DXContainerYAML.cpp
@@ -24,14 +24,14 @@ static_assert((uint64_t)dxbc::FeatureFlags::NextUnusedBit <= 1ull << 63,
               "Shader flag bits exceed enum size.");
 
 DXContainerYAML::ShaderFlags::ShaderFlags(uint64_t FlagData) {
-#define SHADER_FLAG(Num, Val, Str)                                             \
+#define SHADER_FEATURE_INFO(Num, Val, Str)                                             \
   Val = (FlagData & (uint64_t)dxbc::FeatureFlags::Val) > 0;
 #include "llvm/BinaryFormat/DXContainerConstants.def"
 }
 
 uint64_t DXContainerYAML::ShaderFlags::getEncodedFlags() {
   uint64_t Flag = 0;
-#define SHADER_FLAG(Num, Val, Str)                                             \
+#define SHADER_FEATURE_INFO(Num, Val, Str)                                             \
   if (Val)                                                                     \
     Flag |= (uint64_t)dxbc::FeatureFlags::Val;
 #include "llvm/BinaryFormat/DXContainerConstants.def"
@@ -105,7 +105,7 @@ void MappingTraits<DXContainerYAML::DXILProgram>::mapping(
 
 void MappingTraits<DXContainerYAML::ShaderFlags>::mapping(
     IO &IO, DXContainerYAML::ShaderFlags &Flags) {
-#define SHADER_FLAG(Num, Val, Str) IO.mapRequired(#Val, Flags.Val);
+#define SHADER_FEATURE_INFO(Num, Val, Str) IO.mapRequired(#Val, Flags.Val);
 #include "llvm/BinaryFormat/DXContainerConstants.def"
 }
 
diff --git a/llvm/lib/Target/DirectX/DXILShaderFlags.cpp b/llvm/lib/Target/DirectX/DXILShaderFlags.cpp
index bbb56435660211..73606dcef1c874 100644
--- a/llvm/lib/Target/DirectX/DXILShaderFlags.cpp
+++ b/llvm/lib/Target/DirectX/DXILShaderFlags.cpp
@@ -51,7 +51,7 @@ void ComputedShaderFlags::print(raw_ostream &OS) const {
   if (FlagVal == 0)
     return;
   OS << "; Note: shader requires additional functionality:\n";
-#define SHADER_FLAG(bit, FlagName, Str)                                        \
+#define SHADER_FEATURE_INFO(bit, FlagName, Str)                                        \
   if (FlagName)                                                                \
     OS << ";       " Str "\n";
 #include "llvm/BinaryFormat/DXContainerConstants.def"
diff --git a/llvm/lib/Target/DirectX/DXILShaderFlags.h b/llvm/lib/Target/DirectX/DXILShaderFlags.h
index 4f51873a2d0b34..40931be1c51561 100644
--- a/llvm/lib/Target/DirectX/DXILShaderFlags.h
+++ b/llvm/lib/Target/DirectX/DXILShaderFlags.h
@@ -29,17 +29,17 @@ class GlobalVariable;
 namespace dxil {
 
 struct ComputedShaderFlags {
-#define SHADER_FLAG(bit, FlagName, Str) bool FlagName : 1;
+#define SHADER_FEATURE_INFO(bit, FlagName, Str) bool FlagName : 1;
 #include "llvm/BinaryFormat/DXContainerConstants.def"
 
-#define SHADER_FLAG(bit, FlagName, Str) FlagName = false;
+#define SHADER_FEATURE_INFO(bit, FlagName, Str) FlagName = false;
   ComputedShaderFlags() {
 #include "llvm/BinaryFormat/DXContainerConstants.def"
   }
 
   operator uint64_t() const {
     uint64_t FlagValue = 0;
-#define SHADER_FLAG(bit, FlagName, Str)                                        \
+#define SHADER_FEATURE_INFO(bit, FlagName, Str)                                        \
   FlagValue |=                                                                 \
       FlagName ? static_cast<uint64_t>(dxbc::FeatureFlags::FlagName) : 0ull;
 #include "llvm/BinaryFormat/DXContainerConstants.def"

Copy link

github-actions bot commented Feb 22, 2024

✅ With the latest revision this PR passed the C/C++ code formatter.

@@ -24,14 +24,14 @@ static_assert((uint64_t)dxbc::FeatureFlags::NextUnusedBit <= 1ull << 63,
"Shader flag bits exceed enum size.");

DXContainerYAML::ShaderFlags::ShaderFlags(uint64_t FlagData) {
#define SHADER_FLAG(Num, Val, Str) \
#define SHADER_FEATURE_INFO(Num, Val, Str) \
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think this renaming makes this more confusing not less. We probably have a problem that we have too many things called "shader flags", but only partially changing the name makes this more confusing.

A name like "shader feature flags" might be a better name and could be differentiated from the "shader module flags" that are present in the DXIL metadata, but if we're going to do a renaming it should be comprehensive and not leave us in a confusing state.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated to SHADER_FEATURE_FLAG

@@ -56,7 +56,7 @@ struct DXILProgram {
std::optional<std::vector<llvm::yaml::Hex8>> DXIL;
};

#define SHADER_FLAG(Num, Val, Str) bool Val = false;
#define SHADER_FEATURE_FLAG(Num, Val, Str) bool Val = false;
struct ShaderFlags {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should this also be FeatureFlags?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There're 39 DxilModelFlags and 33 FeatureFlags in dxc currently.
Maybe we should save DxilModelFlags and generate FeatureFlags from DxilModelFlags?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm not sure I follow your suggestion.

IMO, the macro should match the way it is used. Without your change we have a SHADER_FLAG macro that generates ShaderFlag structure values. If we're renaming that macro, we should also rename the structure that we generate based on it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated to ShaderFeatureFlags.

Copy link
Contributor

@bob80905 bob80905 left a comment

Choose a reason for hiding this comment

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

Can you change the title of the PR so that it's SHADER_FEATURE_FLAG?

@python3kgae python3kgae changed the title [DirectX][NFC] Rename ShaderFlag to Shader_FEATURE_INFO. [DirectX][NFC] Rename ShaderFlag to SHADER_FEATURE_FLAG. Feb 27, 2024
@python3kgae
Copy link
Contributor Author

Can you change the title of the PR so that it's SHADER_FEATURE_FLAG?

Updated.

@python3kgae python3kgae merged commit 50136ca into llvm:main Feb 28, 2024
3 of 4 checks passed
@python3kgae python3kgae deleted the rename_shader_flag branch February 28, 2024 02:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

None yet

4 participants