addressing comments

Author: joaosaffran

llvm/include/llvm/BinaryFormat/DXContainer.h

@@ -162,7 +162,7 @@ enum class FeatureFlags : uint64_t {
 static_assert((uint64_t)FeatureFlags::NextUnusedBit <= 1ull << 63,
               "Shader flag bits exceed enum size.");
 
-#define ROOT_ELEMENT_FLAG(Num, Val, Str) Val = 1ull << Num,
+#define ROOT_ELEMENT_FLAG(Num, Val) Val = 1ull << Num,
 enum class RootElementFlag : uint32_t {
 #include "DXContainerConstants.def"
 };

llvm/include/llvm/BinaryFormat/DXContainerConstants.def

@@ -53,44 +53,20 @@ SHADER_FEATURE_FLAG(31, 36, NextUnusedBit, "Next reserved shader flag bit (not a
 #undef SHADER_FEATURE_FLAG
 #endif // SHADER_FEATURE_FLAG
 
-#ifdef ROOT_PARAMETER
-
-ROOT_PARAMETER(DescriptorTable)
-ROOT_PARAMETER(Constants32Bit)
-ROOT_PARAMETER(CBV)
-ROOT_PARAMETER(SRV)
-ROOT_PARAMETER(UAV)
-#undef ROOT_PARAMETER
-#endif // ROOT_PARAMETER
-
-
-#ifdef SHADER_VISIBILITY
-
-SHADER_VISIBILITY(All)
-SHADER_VISIBILITY(Vertex)
-SHADER_VISIBILITY(Hull)
-SHADER_VISIBILITY(Domain)
-SHADER_VISIBILITY(Geometry)
-SHADER_VISIBILITY(Pixel)
-SHADER_VISIBILITY(Amplification)
-SHADER_VISIBILITY(Mesh)
-#undef SHADER_VISIBILITY
-#endif // SHADER_VISIBILITY
-
 #ifdef ROOT_ELEMENT_FLAG
 
-ROOT_ELEMENT_FLAG(0, AllowInputAssemblerInputLayout, "The app is opting in to using the Input Assembler")
-ROOT_ELEMENT_FLAG(1, DenyVertexShaderRootAccess, "Denies the vertex shader access to the root signature.")
-ROOT_ELEMENT_FLAG(2, DenyHullShaderRootAccess, "Denies the hull shader access to the root signature.")
-ROOT_ELEMENT_FLAG(3, DenyDomainShaderRootAccess, "Denies the domain shader access to the root signature.")
-ROOT_ELEMENT_FLAG(4, DenyGeometryShaderRootAccess, "Denies the geometry shader access to the root signature.")
-ROOT_ELEMENT_FLAG(5, DenyPixelShaderRootAccess, "Denies the pixel shader access to the root signature.")
-ROOT_ELEMENT_FLAG(6, AllowStreamOutput, "The app is opting in to using Stream Output.")
-ROOT_ELEMENT_FLAG(7, LocalRootSignature, "The root signature is to be used with raytracing shaders to define resource bindings sourced from shader records in shader tables.")
-ROOT_ELEMENT_FLAG(8, DenyAmplificationShaderRootAccess, "Denies the amplification shader access to the root signature.")
-ROOT_ELEMENT_FLAG(9, DenyMeshShaderRootAccess, "Denies the mesh shader access to the root signature.")
-ROOT_ELEMENT_FLAG(10, CBVSRVUAVHeapDirectlyIndexed, "The shaders are allowed to index the CBV/SRV/UAV descriptor heap directly, using the ResourceDescriptorHeap built-in variable.")
-ROOT_ELEMENT_FLAG(11, SamplerHeapDirectlyIndexed, "The shaders are allowed to index the sampler descriptor heap directly, using the SamplerDescriptorHeap built-in variable.")
+ROOT_ELEMENT_FLAG(0, AllowInputAssemblerInputLayout)
+ROOT_ELEMENT_FLAG(1, DenyVertexShaderRootAccess)
+ROOT_ELEMENT_FLAG(2, DenyHullShaderRootAccess)
+ROOT_ELEMENT_FLAG(3, DenyDomainShaderRootAccess)
+ROOT_ELEMENT_FLAG(4, DenyGeometryShaderRootAccess)
+ROOT_ELEMENT_FLAG(5, DenyPixelShaderRootAccess)
+ROOT_ELEMENT_FLAG(6, AllowStreamOutput)
+ROOT_ELEMENT_FLAG(7, LocalRootSignature)
+ROOT_ELEMENT_FLAG(8, DenyAmplificationShaderRootAccess)
+ROOT_ELEMENT_FLAG(9, DenyMeshShaderRootAccess)
+ROOT_ELEMENT_FLAG(10, CBVSRVUAVHeapDirectlyIndexed)
+ROOT_ELEMENT_FLAG(11, SamplerHeapDirectlyIndexed)
 #undef ROOT_ELEMENT_FLAG
 #endif // ROOT_ELEMENT_FLAG
 

llvm/include/llvm/ObjectYAML/DXContainerYAML.h

@@ -73,18 +73,19 @@ struct ShaderHash {
   std::vector<llvm::yaml::Hex8> Digest;
 };
 
-#define ROOT_ELEMENT_FLAG(Num, Val, Str) bool Val = false;
+#define ROOT_ELEMENT_FLAG(Num, Val) bool Val = false;
 struct RootSignatureDesc {
   RootSignatureDesc() = default;
   RootSignatureDesc(const object::DirectX::RootSignature &Data);
 
-  uint32_t getEncodedFlags();
   uint32_t Version;
   uint32_t NumParameters;
   uint32_t RootParametersOffset;
   uint32_t NumStaticSamplers;
   uint32_t StaticSamplersOffset;
 
+  uint32_t getEncodedFlags();
+
 #include "llvm/BinaryFormat/DXContainerConstants.def"
 };
 

llvm/lib/ObjectYAML/DXContainerEmitter.cpp

@@ -268,6 +268,11 @@ void DXContainerWriter::writeParts(raw_ostream &OS) {
 
       mcdxbc::RootSignatureHeader Header;
       Header.Flags = P.RootSignature->getEncodedFlags();
+      Header.Version = P.RootSignature->Version;
+      Header.NumParameters = P.RootSignature->NumParameters;
+      Header.RootParametersOffset = P.RootSignature->RootParametersOffset;
+      Header.NumStaticSamplers = P.RootSignature->NumStaticSamplers;
+      Header.StaticSamplersOffset = P.RootSignature->StaticSamplersOffset;
 
       Header.write(OS);
       break;

llvm/lib/ObjectYAML/DXContainerYAML.cpp

@@ -37,14 +37,14 @@ DXContainerYAML::RootSignatureDesc::RootSignatureDesc(
       NumStaticSamplers(Data.getNumStaticSamplers()),
       StaticSamplersOffset(Data.getStaticSamplersOffset()) {
   uint32_t Flags = Data.getFlags();
-#define ROOT_ELEMENT_FLAG(Num, Val, Str)                                       \
+#define ROOT_ELEMENT_FLAG(Num, Val)                                            \
   Val = (Flags & (uint32_t)dxbc::RootElementFlag::Val) > 0;
 #include "llvm/BinaryFormat/DXContainerConstants.def"
 }
 
 uint32_t DXContainerYAML::RootSignatureDesc::getEncodedFlags() {
   uint64_t Flag = 0;
-#define ROOT_ELEMENT_FLAG(Num, Val, Str)                                       \
+#define ROOT_ELEMENT_FLAG(Num, Val)                                            \
   if (Val)                                                                     \
     Flag |= (uint32_t)dxbc::RootElementFlag::Val;
 #include "llvm/BinaryFormat/DXContainerConstants.def"
@@ -217,7 +217,7 @@ void MappingTraits<DXContainerYAML::RootSignatureDesc>::mapping(
   IO.mapRequired("RootParametersOffset", S.RootParametersOffset);
   IO.mapRequired("NumStaticSamplers", S.NumStaticSamplers);
   IO.mapRequired("StaticSamplersOffset", S.StaticSamplersOffset);
-#define ROOT_ELEMENT_FLAG(Num, Val, Str) IO.mapOptional(#Val, S.Val, false);
+#define ROOT_ELEMENT_FLAG(Num, Val) IO.mapOptional(#Val, S.Val, false);
 #include "llvm/BinaryFormat/DXContainerConstants.def"
 }
 

llvm/test/ObjectYAML/DXContainer/RootSignature-Flags.yaml

@@ -14,18 +14,20 @@ Parts:
     Size:            24
     RootSignature:
       Version: 2
-      NumParameters: 0
-      RootParametersOffset: 0
-      NumStaticSamplers: 0
-      StaticSamplersOffset: 0
+      NumParameters: 1
+      RootParametersOffset: 3
+      NumStaticSamplers: 4
+      StaticSamplersOffset: 5
       AllowInputAssemblerInputLayout: true
+      DenyGeometryShaderRootAccess: true
 
 # CHECK:  - Name:            RTS0
 # CHECK-NEXT:    Size:            24
 # CHECK-NEXT:    RootSignature:
 # CHECK-NEXT:      Version: 2
-# CHECK-NEXT:      NumParameters: 0
-# CHECK-NEXT:      RootParametersOffset: 0
-# CHECK-NEXT:      NumStaticSamplers: 0
-# CHECK-NEXT:      StaticSamplersOffset: 0
+# CHECK-NEXT:      NumParameters: 1
+# CHECK-NEXT:      RootParametersOffset: 3
+# CHECK-NEXT:      NumStaticSamplers: 4
+# CHECK-NEXT:      StaticSamplersOffset: 5
 # CHECK-NEXT:      AllowInputAssemblerInputLayout: true
+# CHECK-NEXT:      DenyGeometryShaderRootAccess: true

llvm/unittests/ObjectYAML/DXContainerYAMLTest.cpp

@@ -128,9 +128,9 @@ TEST(RootSignature, ParseRootFlags) {
       RootSignature:
         Version:         2
         NumParameters:   0
-        RootParametersOffset: 24
+        RootParametersOffset: 0
         NumStaticSamplers: 0
-        StaticSamplersOffset: 24
+        StaticSamplersOffset: 0
         AllowInputAssemblerInputLayout: true
     )"));