[llvm-branch-commits] [llvm] [HLSL] Adding support for Root Constants in LLVM Metadata (PR #135085)

via llvm-branch-commits llvm-branch-commits at lists.llvm.org
Mon Apr 14 11:57:57 PDT 2025


https://github.com/joaosaffran updated https://github.com/llvm/llvm-project/pull/135085

>From 9b59d0108f6b23c039e2c417247216862073cd4b Mon Sep 17 00:00:00 2001
From: joaosaffran <joao.saffran at microsoft.com>
Date: Wed, 9 Apr 2025 21:05:58 +0000
Subject: [PATCH 1/9] adding support for root constants in metadata generation

---
 llvm/lib/Target/DirectX/DXILRootSignature.cpp | 120 +++++++++++++++++-
 llvm/lib/Target/DirectX/DXILRootSignature.h   |   6 +-
 .../RootSignature-Flags-Validation-Error.ll   |   7 +-
 .../RootSignature-RootConstants.ll            |  34 +++++
 ...ature-ShaderVisibility-Validation-Error.ll |  20 +++
 5 files changed, 182 insertions(+), 5 deletions(-)
 create mode 100644 llvm/test/CodeGen/DirectX/ContainerData/RootSignature-RootConstants.ll
 create mode 100644 llvm/test/CodeGen/DirectX/ContainerData/RootSignature-ShaderVisibility-Validation-Error.ll

diff --git a/llvm/lib/Target/DirectX/DXILRootSignature.cpp b/llvm/lib/Target/DirectX/DXILRootSignature.cpp
index 412ab7765a7ae..7686918b0fc75 100644
--- a/llvm/lib/Target/DirectX/DXILRootSignature.cpp
+++ b/llvm/lib/Target/DirectX/DXILRootSignature.cpp
@@ -40,6 +40,13 @@ static bool reportError(LLVMContext *Ctx, Twine Message,
   return true;
 }
 
+static bool reportValueError(LLVMContext *Ctx, Twine ParamName, uint32_t Value,
+                             DiagnosticSeverity Severity = DS_Error) {
+  Ctx->diagnose(DiagnosticInfoGeneric(
+      "Invalid value for " + ParamName + ": " + Twine(Value), Severity));
+  return true;
+}
+
 static bool parseRootFlags(LLVMContext *Ctx, mcdxbc::RootSignatureDesc &RSD,
                            MDNode *RootFlagNode) {
 
@@ -52,6 +59,45 @@ static bool parseRootFlags(LLVMContext *Ctx, mcdxbc::RootSignatureDesc &RSD,
   return false;
 }
 
+static bool extractMdValue(uint32_t &Value, MDNode *Node, unsigned int OpId) {
+
+  auto *CI = mdconst::extract<ConstantInt>(Node->getOperand(OpId));
+  if (CI == nullptr)
+    return true;
+
+  Value = CI->getZExtValue();
+  return false;
+}
+
+static bool parseRootConstants(LLVMContext *Ctx, mcdxbc::RootSignatureDesc &RSD,
+                               MDNode *RootFlagNode) {
+
+  if (RootFlagNode->getNumOperands() != 5)
+    return reportError(Ctx, "Invalid format for RootConstants Element");
+
+  mcdxbc::RootParameter NewParameter;
+  NewParameter.Header.ParameterType = dxbc::RootParameterType::Constants32Bit;
+
+  uint32_t SV;
+  if (extractMdValue(SV, RootFlagNode, 1))
+    return reportError(Ctx, "Invalid value for ShaderVisibility");
+
+  NewParameter.Header.ShaderVisibility = (dxbc::ShaderVisibility)SV;
+
+  if (extractMdValue(NewParameter.Constants.ShaderRegister, RootFlagNode, 2))
+    return reportError(Ctx, "Invalid value for ShaderRegister");
+
+  if (extractMdValue(NewParameter.Constants.RegisterSpace, RootFlagNode, 3))
+    return reportError(Ctx, "Invalid value for RegisterSpace");
+
+  if (extractMdValue(NewParameter.Constants.Num32BitValues, RootFlagNode, 4))
+    return reportError(Ctx, "Invalid value for Num32BitValues");
+
+  RSD.Parameters.push_back(NewParameter);
+
+  return false;
+}
+
 static bool parseRootSignatureElement(LLVMContext *Ctx,
                                       mcdxbc::RootSignatureDesc &RSD,
                                       MDNode *Element) {
@@ -62,12 +108,16 @@ static bool parseRootSignatureElement(LLVMContext *Ctx,
   RootSignatureElementKind ElementKind =
       StringSwitch<RootSignatureElementKind>(ElementText->getString())
           .Case("RootFlags", RootSignatureElementKind::RootFlags)
+          .Case("RootConstants", RootSignatureElementKind::RootConstants)
           .Default(RootSignatureElementKind::Error);
 
   switch (ElementKind) {
 
   case RootSignatureElementKind::RootFlags:
     return parseRootFlags(Ctx, RSD, Element);
+  case RootSignatureElementKind::RootConstants:
+    return parseRootConstants(Ctx, RSD, Element);
+    break;
   case RootSignatureElementKind::Error:
     return reportError(Ctx, "Invalid Root Signature Element: " +
                                 ElementText->getString());
@@ -94,10 +144,56 @@ static bool parse(LLVMContext *Ctx, mcdxbc::RootSignatureDesc &RSD,
 
 static bool verifyRootFlag(uint32_t Flags) { return (Flags & ~0xfff) == 0; }
 
+static bool verifyShaderVisibility(dxbc::ShaderVisibility Flags) {
+  switch (Flags) {
+
+  case dxbc::ShaderVisibility::All:
+  case dxbc::ShaderVisibility::Vertex:
+  case dxbc::ShaderVisibility::Hull:
+  case dxbc::ShaderVisibility::Domain:
+  case dxbc::ShaderVisibility::Geometry:
+  case dxbc::ShaderVisibility::Pixel:
+  case dxbc::ShaderVisibility::Amplification:
+  case dxbc::ShaderVisibility::Mesh:
+    return true;
+  }
+
+  return false;
+}
+
+static bool verifyParameterType(dxbc::RootParameterType Flags) {
+  switch (Flags) {
+  case dxbc::RootParameterType::Constants32Bit:
+    return true;
+  }
+
+  return false;
+}
+
+static bool verifyVersion(uint32_t Version) {
+  return (Version == 1 || Version == 2);
+}
+
 static bool validate(LLVMContext *Ctx, const mcdxbc::RootSignatureDesc &RSD) {
+
+  if (!verifyVersion(RSD.Header.Version)) {
+    return reportValueError(Ctx, "Version", RSD.Header.Version);
+  }
+
   if (!verifyRootFlag(RSD.Header.Flags)) {
-    return reportError(Ctx, "Invalid Root Signature flag value");
+    return reportValueError(Ctx, "RootFlags", RSD.Header.Flags);
+  }
+
+  for (const auto &P : RSD.Parameters) {
+    if (!verifyShaderVisibility(P.Header.ShaderVisibility))
+      return reportValueError(Ctx, "ShaderVisibility",
+                              (uint32_t)P.Header.ShaderVisibility);
+
+    if (!verifyParameterType(P.Header.ParameterType))
+      return reportValueError(Ctx, "ParameterType",
+                              (uint32_t)P.Header.ParameterType);
   }
+
   return false;
 }
 
@@ -211,6 +307,28 @@ PreservedAnalyses RootSignatureAnalysisPrinter::run(Module &M,
     OS << indent(Space) << "NumStaticSamplers: " << 0 << ":\n";
     OS << indent(Space) << "StaticSamplersOffset: "
        << sizeof(RS.Header) + RS.Parameters.size_in_bytes() << ":\n";
+
+    Space++;
+    for (auto const &P : RS.Parameters) {
+      OS << indent(Space) << "Parameter Type: " << &P.Header.ParameterType
+         << ":\n";
+      OS << indent(Space) << "Shader Visibility: " << &P.Header.ShaderVisibility
+         << ":\n";
+      OS << indent(Space) << "Parameter Offset: " << &P.Header.ParameterOffset
+         << ":\n";
+      switch (P.Header.ParameterType) {
+      case dxbc::RootParameterType::Constants32Bit:
+        OS << indent(Space) << "Register Space: " << &P.Constants.RegisterSpace
+           << ":\n";
+        OS << indent(Space)
+           << "Shader Register: " << &P.Constants.ShaderRegister << ":\n";
+        OS << indent(Space)
+           << "Num 32 Bit Values: " << &P.Constants.Num32BitValues << ":\n";
+        break;
+      }
+    }
+    Space--;
+
     Space--;
     // end root signature header
   }
diff --git a/llvm/lib/Target/DirectX/DXILRootSignature.h b/llvm/lib/Target/DirectX/DXILRootSignature.h
index 8c25b2eb3fadf..93ec614f1ab85 100644
--- a/llvm/lib/Target/DirectX/DXILRootSignature.h
+++ b/llvm/lib/Target/DirectX/DXILRootSignature.h
@@ -24,7 +24,11 @@
 namespace llvm {
 namespace dxil {
 
-enum class RootSignatureElementKind { Error = 0, RootFlags = 1 };
+enum class RootSignatureElementKind {
+  Error = 0,
+  RootFlags = 1,
+  RootConstants = 2
+};
 class RootSignatureAnalysis : public AnalysisInfoMixin<RootSignatureAnalysis> {
   friend AnalysisInfoMixin<RootSignatureAnalysis>;
   static AnalysisKey Key;
diff --git a/llvm/test/CodeGen/DirectX/ContainerData/RootSignature-Flags-Validation-Error.ll b/llvm/test/CodeGen/DirectX/ContainerData/RootSignature-Flags-Validation-Error.ll
index fe93c9993c1c3..3af9a524e77f4 100644
--- a/llvm/test/CodeGen/DirectX/ContainerData/RootSignature-Flags-Validation-Error.ll
+++ b/llvm/test/CodeGen/DirectX/ContainerData/RootSignature-Flags-Validation-Error.ll
@@ -1,6 +1,6 @@
 ; RUN: not opt -passes='print<dxil-root-signature>' %s -S -o - 2>&1 | FileCheck %s
 
-; CHECK: error: Invalid Root Signature flag value
+; CHECK: error: Invalid value for ShaderVisibility: 255
 ; CHECK-NOT: Root Signature Definitions
 
 target triple = "dxil-unknown-shadermodel6.0-compute"
@@ -16,5 +16,6 @@ attributes #0 = { "hlsl.numthreads"="1,1,1" "hlsl.shader"="compute" }
 
 !dx.rootsignatures = !{!2} ; list of function/root signature pairs
 !2 = !{ ptr @main, !3 } ; function, root signature
-!3 = !{ !4 } ; list of root signature elements
-!4 = !{ !"RootFlags", i32 2147487744 } ; 1 = allow_input_assembler_input_layout
+!3 = !{ !4, !5 } ; list of root signature elements
+!4 = !{ !"RootFlags", i32 1 } ; 1 = allow_input_assembler_input_layout
+!5 = !{ !"RootConstants", i32 255, i32 1, i32 2, i32 3 }
diff --git a/llvm/test/CodeGen/DirectX/ContainerData/RootSignature-RootConstants.ll b/llvm/test/CodeGen/DirectX/ContainerData/RootSignature-RootConstants.ll
new file mode 100644
index 0000000000000..d0520632df2a2
--- /dev/null
+++ b/llvm/test/CodeGen/DirectX/ContainerData/RootSignature-RootConstants.ll
@@ -0,0 +1,34 @@
+; RUN: opt %s -dxil-embed -dxil-globals -S -o - | FileCheck %s
+; RUN: llc %s --filetype=obj -o - | obj2yaml | FileCheck %s --check-prefix=DXC
+
+target triple = "dxil-unknown-shadermodel6.0-compute"
+
+; CHECK: @dx.rts0 = private constant [48 x i8]  c"{{.*}}", section "RTS0", align 4
+
+define void @main() #0 {
+entry:
+  ret void
+}
+attributes #0 = { "hlsl.numthreads"="1,1,1" "hlsl.shader"="compute" }
+
+
+!dx.rootsignatures = !{!2} ; list of function/root signature pairs
+!2 = !{ ptr @main, !3 } ; function, root signature
+!3 = !{ !4, !5 } ; list of root signature elements
+!4 = !{ !"RootFlags", i32 1 } ; 1 = allow_input_assembler_input_layout
+!5 = !{ !"RootConstants", i32 0, i32 1, i32 2, i32 3 }
+
+; DXC:  - Name:            RTS0
+; DXC-NEXT:    Size:            48
+; DXC-NEXT:    RootSignature:
+; DXC-NEXT:      Version:         2
+; DXC-NEXT:      NumStaticSamplers: 0
+; DXC-NEXT:      StaticSamplersOffset: 0
+; DXC-NEXT:      Parameters:
+; DXC-NEXT:        - ParameterType:   Constants32Bit
+; DXC-NEXT:          ShaderVisibility: All
+; DXC-NEXT:          Constants:
+; DXC-NEXT:            Num32BitValues:  3
+; DXC-NEXT:            RegisterSpace:   2
+; DXC-NEXT:            ShaderRegister:  1
+; DXC-NEXT:      AllowInputAssemblerInputLayout: true
diff --git a/llvm/test/CodeGen/DirectX/ContainerData/RootSignature-ShaderVisibility-Validation-Error.ll b/llvm/test/CodeGen/DirectX/ContainerData/RootSignature-ShaderVisibility-Validation-Error.ll
new file mode 100644
index 0000000000000..4b8e6abacd7ad
--- /dev/null
+++ b/llvm/test/CodeGen/DirectX/ContainerData/RootSignature-ShaderVisibility-Validation-Error.ll
@@ -0,0 +1,20 @@
+; RUN: not opt -passes='print<dxil-root-signature>' %s -S -o - 2>&1 | FileCheck %s
+
+; CHECK: error: Invalid value for RootFlags: 2147487744
+; CHECK-NOT: Root Signature Definitions
+
+target triple = "dxil-unknown-shadermodel6.0-compute"
+
+
+define void @main() #0 {
+entry:
+  ret void
+}
+
+attributes #0 = { "hlsl.numthreads"="1,1,1" "hlsl.shader"="compute" }
+
+
+!dx.rootsignatures = !{!2} ; list of function/root signature pairs
+!2 = !{ ptr @main, !3 } ; function, root signature
+!3 = !{ !4 } ; list of root signature elements
+!4 = !{ !"RootFlags", i32 2147487744 } ; 1 = allow_input_assembler_input_layout

>From efc5e52bb8843a025f49e41b322682753c061c3f Mon Sep 17 00:00:00 2001
From: joaosaffran <joao.saffran at microsoft.com>
Date: Wed, 9 Apr 2025 21:19:25 +0000
Subject: [PATCH 2/9] add test

---
 llvm/lib/Target/DirectX/DXILRootSignature.cpp | 33 +++++++++----------
 .../ContainerData/RootSignature-Parameters.ll | 30 +++++++++++++++++
 2 files changed, 46 insertions(+), 17 deletions(-)
 create mode 100644 llvm/test/CodeGen/DirectX/ContainerData/RootSignature-Parameters.ll

diff --git a/llvm/lib/Target/DirectX/DXILRootSignature.cpp b/llvm/lib/Target/DirectX/DXILRootSignature.cpp
index 7686918b0fc75..7f30cef239696 100644
--- a/llvm/lib/Target/DirectX/DXILRootSignature.cpp
+++ b/llvm/lib/Target/DirectX/DXILRootSignature.cpp
@@ -299,31 +299,30 @@ PreservedAnalyses RootSignatureAnalysisPrinter::run(Module &M,
 
     // start root signature header
     Space++;
-    OS << indent(Space) << "Flags: " << format_hex(RS.Header.Flags, 8) << ":\n";
-    OS << indent(Space) << "Version: " << RS.Header.Version << ":\n";
-    OS << indent(Space) << "NumParameters: " << RS.Parameters.size() << ":\n";
+    OS << indent(Space) << "Flags: " << format_hex(RS.Header.Flags, 8) << "\n";
+    OS << indent(Space) << "Version: " << RS.Header.Version << "\n";
+    OS << indent(Space) << "NumParameters: " << RS.Parameters.size() << "\n";
     OS << indent(Space) << "RootParametersOffset: " << sizeof(RS.Header)
-       << ":\n";
-    OS << indent(Space) << "NumStaticSamplers: " << 0 << ":\n";
+       << "\n";
+    OS << indent(Space) << "NumStaticSamplers: " << 0 << "\n";
     OS << indent(Space) << "StaticSamplersOffset: "
-       << sizeof(RS.Header) + RS.Parameters.size_in_bytes() << ":\n";
+       << sizeof(RS.Header) + RS.Parameters.size_in_bytes() << "\n";
 
     Space++;
     for (auto const &P : RS.Parameters) {
-      OS << indent(Space) << "Parameter Type: " << &P.Header.ParameterType
-         << ":\n";
-      OS << indent(Space) << "Shader Visibility: " << &P.Header.ShaderVisibility
-         << ":\n";
-      OS << indent(Space) << "Parameter Offset: " << &P.Header.ParameterOffset
-         << ":\n";
+      OS << indent(Space)
+         << "Parameter Type: " << (uint32_t)P.Header.ParameterType << "\n";
+      OS << indent(Space)
+         << "Shader Visibility: " << (uint32_t)P.Header.ShaderVisibility
+         << "\n";
       switch (P.Header.ParameterType) {
       case dxbc::RootParameterType::Constants32Bit:
-        OS << indent(Space) << "Register Space: " << &P.Constants.RegisterSpace
-           << ":\n";
+        OS << indent(Space) << "Register Space: " << P.Constants.RegisterSpace
+           << "\n";
+        OS << indent(Space) << "Shader Register: " << P.Constants.ShaderRegister
+           << "\n";
         OS << indent(Space)
-           << "Shader Register: " << &P.Constants.ShaderRegister << ":\n";
-        OS << indent(Space)
-           << "Num 32 Bit Values: " << &P.Constants.Num32BitValues << ":\n";
+           << "Num 32 Bit Values: " << P.Constants.Num32BitValues << "\n";
         break;
       }
     }
diff --git a/llvm/test/CodeGen/DirectX/ContainerData/RootSignature-Parameters.ll b/llvm/test/CodeGen/DirectX/ContainerData/RootSignature-Parameters.ll
new file mode 100644
index 0000000000000..9a2f7d840a236
--- /dev/null
+++ b/llvm/test/CodeGen/DirectX/ContainerData/RootSignature-Parameters.ll
@@ -0,0 +1,30 @@
+; RUN: opt -passes='print<dxil-root-signature>' %s -S -o - 2>&1 | FileCheck %s
+
+target triple = "dxil-unknown-shadermodel6.0-compute"
+
+
+define void @main() #0 {
+entry:
+  ret void
+}
+
+attributes #0 = { "hlsl.numthreads"="1,1,1" "hlsl.shader"="compute" }
+
+!dx.rootsignatures = !{!2} ; list of function/root signature pairs
+!2 = !{ ptr @main, !3 } ; function, root signature
+!3 = !{ !4, !5 } ; list of root signature elements
+!4 = !{ !"RootFlags", i32 1 } ; 1 = allow_input_assembler_input_layout
+!5 = !{ !"RootConstants", i32 0, i32 1, i32 2, i32 3 }
+
+; CHECK-LABEL: Definition for 'main':
+; CHECK-NEXT: Flags: 0x000001
+; CHECK-NEXT: Version: 2
+; CHECK-NEXT: NumParameters: 1
+; CHECK-NEXT: RootParametersOffset: 24
+; CHECK-NEXT: NumStaticSamplers: 0
+; CHECK-NEXT: StaticSamplersOffset: 48
+; CHECK-NEXT:  Parameter Type: 1
+; CHECK-NEXT:  Shader Visibility: 0
+; CHECK-NEXT:  Register Space: 2
+; CHECK-NEXT:  Shader Register: 1
+; CHECK-NEXT:  Num 32 Bit Values: 3

>From e6865a76f73e172304f64d1bdd7225f7ff33a449 Mon Sep 17 00:00:00 2001
From: joaosaffran <joao.saffran at microsoft.com>
Date: Wed, 9 Apr 2025 23:37:34 +0000
Subject: [PATCH 3/9] Revert "clean up"

This reverts commit cfc6bfb3fcb5a1739b4c304a273d8ce4cd651c56.
---
 llvm/include/llvm/BinaryFormat/DXContainer.h    | 2 +-
 llvm/include/llvm/MC/DXContainerRootSignature.h | 1 +
 llvm/include/llvm/Object/DXContainer.h          | 3 +++
 llvm/lib/MC/DXContainerRootSignature.cpp        | 1 +
 llvm/lib/Object/DXContainer.cpp                 | 2 ++
 llvm/lib/ObjectYAML/DXContainerYAML.cpp         | 3 +++
 llvm/unittests/Object/DXContainerTest.cpp       | 3 +++
 7 files changed, 14 insertions(+), 1 deletion(-)

diff --git a/llvm/include/llvm/BinaryFormat/DXContainer.h b/llvm/include/llvm/BinaryFormat/DXContainer.h
index e0a03d6d777a8..f9140b6a8c805 100644
--- a/llvm/include/llvm/BinaryFormat/DXContainer.h
+++ b/llvm/include/llvm/BinaryFormat/DXContainer.h
@@ -55,7 +55,7 @@ enum class HashFlags : uint32_t {
 };
 
 struct ShaderHash {
-  uint32_t Flags; // dxbc::HashFlags
+  uint32_t Flags; // HashFlags
   uint8_t Digest[16];
 
   bool isPopulated();
diff --git a/llvm/include/llvm/MC/DXContainerRootSignature.h b/llvm/include/llvm/MC/DXContainerRootSignature.h
index 0ad7b21981415..6cf210728c7e8 100644
--- a/llvm/include/llvm/MC/DXContainerRootSignature.h
+++ b/llvm/include/llvm/MC/DXContainerRootSignature.h
@@ -7,6 +7,7 @@
 //===----------------------------------------------------------------------===//
 
 #include "llvm/BinaryFormat/DXContainer.h"
+#include "llvm/Support/ErrorHandling.h"
 #include <cstdint>
 #include <limits>
 
diff --git a/llvm/include/llvm/Object/DXContainer.h b/llvm/include/llvm/Object/DXContainer.h
index 5a20679f05c19..cf7a9d5be93ae 100644
--- a/llvm/include/llvm/Object/DXContainer.h
+++ b/llvm/include/llvm/Object/DXContainer.h
@@ -23,6 +23,9 @@
 #include "llvm/Support/MemoryBufferRef.h"
 #include "llvm/TargetParser/Triple.h"
 #include <array>
+#include <cstddef>
+#include <cstdint>
+#include <memory>
 #include <variant>
 
 namespace llvm {
diff --git a/llvm/lib/MC/DXContainerRootSignature.cpp b/llvm/lib/MC/DXContainerRootSignature.cpp
index a846e4572c449..e5f67df21e8ef 100644
--- a/llvm/lib/MC/DXContainerRootSignature.cpp
+++ b/llvm/lib/MC/DXContainerRootSignature.cpp
@@ -8,6 +8,7 @@
 
 #include "llvm/MC/DXContainerRootSignature.h"
 #include "llvm/ADT/SmallString.h"
+#include "llvm/BinaryFormat/DXContainer.h"
 #include "llvm/Support/EndianStream.h"
 
 using namespace llvm;
diff --git a/llvm/lib/Object/DXContainer.cpp b/llvm/lib/Object/DXContainer.cpp
index 2e014dd0c0c53..04d124b6bb302 100644
--- a/llvm/lib/Object/DXContainer.cpp
+++ b/llvm/lib/Object/DXContainer.cpp
@@ -11,7 +11,9 @@
 #include "llvm/Object/Error.h"
 #include "llvm/Support/Alignment.h"
 #include "llvm/Support/Endian.h"
+#include "llvm/Support/Error.h"
 #include "llvm/Support/FormatVariadic.h"
+#include <cstdint>
 
 using namespace llvm;
 using namespace llvm::object;
diff --git a/llvm/lib/ObjectYAML/DXContainerYAML.cpp b/llvm/lib/ObjectYAML/DXContainerYAML.cpp
index e8578df8aa509..3380c80d130de 100644
--- a/llvm/lib/ObjectYAML/DXContainerYAML.cpp
+++ b/llvm/lib/ObjectYAML/DXContainerYAML.cpp
@@ -14,6 +14,9 @@
 #include "llvm/ObjectYAML/DXContainerYAML.h"
 #include "llvm/ADT/ScopeExit.h"
 #include "llvm/BinaryFormat/DXContainer.h"
+#include "llvm/Object/DXContainer.h"
+#include "llvm/Support/Casting.h"
+#include "llvm/Support/Error.h"
 #include "llvm/Support/ScopedPrinter.h"
 
 namespace llvm {
diff --git a/llvm/unittests/Object/DXContainerTest.cpp b/llvm/unittests/Object/DXContainerTest.cpp
index b2ff9ad218de0..03d3d6d73bbdc 100644
--- a/llvm/unittests/Object/DXContainerTest.cpp
+++ b/llvm/unittests/Object/DXContainerTest.cpp
@@ -8,9 +8,12 @@
 
 #include "llvm/Object/DXContainer.h"
 #include "llvm/ADT/StringRef.h"
+#include "llvm/BinaryFormat/DXContainer.h"
 #include "llvm/BinaryFormat/Magic.h"
 #include "llvm/ObjectYAML/DXContainerYAML.h"
 #include "llvm/ObjectYAML/yaml2obj.h"
+#include "llvm/Support/Casting.h"
+#include "llvm/Support/Error.h"
 #include "llvm/Support/MemoryBufferRef.h"
 #include "llvm/Testing/Support/Error.h"
 #include "gtest/gtest.h"

>From 021976d8a2e3836c940a7b7b3acd0f5ed0c044c1 Mon Sep 17 00:00:00 2001
From: joaosaffran <joao.saffran at microsoft.com>
Date: Wed, 9 Apr 2025 23:39:48 +0000
Subject: [PATCH 4/9] Reapply "clean up"

This reverts commit e6865a76f73e172304f64d1bdd7225f7ff33a449.
---
 llvm/include/llvm/BinaryFormat/DXContainer.h    | 2 +-
 llvm/include/llvm/MC/DXContainerRootSignature.h | 1 -
 llvm/include/llvm/Object/DXContainer.h          | 3 ---
 llvm/lib/MC/DXContainerRootSignature.cpp        | 1 -
 llvm/lib/Object/DXContainer.cpp                 | 2 --
 llvm/lib/ObjectYAML/DXContainerYAML.cpp         | 3 ---
 llvm/unittests/Object/DXContainerTest.cpp       | 3 ---
 7 files changed, 1 insertion(+), 14 deletions(-)

diff --git a/llvm/include/llvm/BinaryFormat/DXContainer.h b/llvm/include/llvm/BinaryFormat/DXContainer.h
index f9140b6a8c805..e0a03d6d777a8 100644
--- a/llvm/include/llvm/BinaryFormat/DXContainer.h
+++ b/llvm/include/llvm/BinaryFormat/DXContainer.h
@@ -55,7 +55,7 @@ enum class HashFlags : uint32_t {
 };
 
 struct ShaderHash {
-  uint32_t Flags; // HashFlags
+  uint32_t Flags; // dxbc::HashFlags
   uint8_t Digest[16];
 
   bool isPopulated();
diff --git a/llvm/include/llvm/MC/DXContainerRootSignature.h b/llvm/include/llvm/MC/DXContainerRootSignature.h
index 6cf210728c7e8..0ad7b21981415 100644
--- a/llvm/include/llvm/MC/DXContainerRootSignature.h
+++ b/llvm/include/llvm/MC/DXContainerRootSignature.h
@@ -7,7 +7,6 @@
 //===----------------------------------------------------------------------===//
 
 #include "llvm/BinaryFormat/DXContainer.h"
-#include "llvm/Support/ErrorHandling.h"
 #include <cstdint>
 #include <limits>
 
diff --git a/llvm/include/llvm/Object/DXContainer.h b/llvm/include/llvm/Object/DXContainer.h
index cf7a9d5be93ae..5a20679f05c19 100644
--- a/llvm/include/llvm/Object/DXContainer.h
+++ b/llvm/include/llvm/Object/DXContainer.h
@@ -23,9 +23,6 @@
 #include "llvm/Support/MemoryBufferRef.h"
 #include "llvm/TargetParser/Triple.h"
 #include <array>
-#include <cstddef>
-#include <cstdint>
-#include <memory>
 #include <variant>
 
 namespace llvm {
diff --git a/llvm/lib/MC/DXContainerRootSignature.cpp b/llvm/lib/MC/DXContainerRootSignature.cpp
index e5f67df21e8ef..a846e4572c449 100644
--- a/llvm/lib/MC/DXContainerRootSignature.cpp
+++ b/llvm/lib/MC/DXContainerRootSignature.cpp
@@ -8,7 +8,6 @@
 
 #include "llvm/MC/DXContainerRootSignature.h"
 #include "llvm/ADT/SmallString.h"
-#include "llvm/BinaryFormat/DXContainer.h"
 #include "llvm/Support/EndianStream.h"
 
 using namespace llvm;
diff --git a/llvm/lib/Object/DXContainer.cpp b/llvm/lib/Object/DXContainer.cpp
index 04d124b6bb302..2e014dd0c0c53 100644
--- a/llvm/lib/Object/DXContainer.cpp
+++ b/llvm/lib/Object/DXContainer.cpp
@@ -11,9 +11,7 @@
 #include "llvm/Object/Error.h"
 #include "llvm/Support/Alignment.h"
 #include "llvm/Support/Endian.h"
-#include "llvm/Support/Error.h"
 #include "llvm/Support/FormatVariadic.h"
-#include <cstdint>
 
 using namespace llvm;
 using namespace llvm::object;
diff --git a/llvm/lib/ObjectYAML/DXContainerYAML.cpp b/llvm/lib/ObjectYAML/DXContainerYAML.cpp
index 3380c80d130de..e8578df8aa509 100644
--- a/llvm/lib/ObjectYAML/DXContainerYAML.cpp
+++ b/llvm/lib/ObjectYAML/DXContainerYAML.cpp
@@ -14,9 +14,6 @@
 #include "llvm/ObjectYAML/DXContainerYAML.h"
 #include "llvm/ADT/ScopeExit.h"
 #include "llvm/BinaryFormat/DXContainer.h"
-#include "llvm/Object/DXContainer.h"
-#include "llvm/Support/Casting.h"
-#include "llvm/Support/Error.h"
 #include "llvm/Support/ScopedPrinter.h"
 
 namespace llvm {
diff --git a/llvm/unittests/Object/DXContainerTest.cpp b/llvm/unittests/Object/DXContainerTest.cpp
index 03d3d6d73bbdc..b2ff9ad218de0 100644
--- a/llvm/unittests/Object/DXContainerTest.cpp
+++ b/llvm/unittests/Object/DXContainerTest.cpp
@@ -8,12 +8,9 @@
 
 #include "llvm/Object/DXContainer.h"
 #include "llvm/ADT/StringRef.h"
-#include "llvm/BinaryFormat/DXContainer.h"
 #include "llvm/BinaryFormat/Magic.h"
 #include "llvm/ObjectYAML/DXContainerYAML.h"
 #include "llvm/ObjectYAML/yaml2obj.h"
-#include "llvm/Support/Casting.h"
-#include "llvm/Support/Error.h"
 #include "llvm/Support/MemoryBufferRef.h"
 #include "llvm/Testing/Support/Error.h"
 #include "gtest/gtest.h"

>From 102ff4bea5846b6737884e6f7114f5e223e9fa35 Mon Sep 17 00:00:00 2001
From: joaosaffran <joao.saffran at microsoft.com>
Date: Thu, 10 Apr 2025 00:09:08 +0000
Subject: [PATCH 5/9] removing unnecessary parameters

---
 llvm/include/llvm/Object/DXContainer.h | 18 ++++++++----------
 llvm/lib/Object/DXContainer.cpp        | 10 +---------
 2 files changed, 9 insertions(+), 19 deletions(-)

diff --git a/llvm/include/llvm/Object/DXContainer.h b/llvm/include/llvm/Object/DXContainer.h
index 5a20679f05c19..eeee443cb9460 100644
--- a/llvm/include/llvm/Object/DXContainer.h
+++ b/llvm/include/llvm/Object/DXContainer.h
@@ -170,13 +170,12 @@ class RootSignature {
   uint32_t StaticSamplersOffset;
   uint32_t Flags;
   ViewArray<dxbc::RootParameterHeader> ParametersHeaders;
-  uint32_t ParameterSpaceOffset;
-  StringRef ParameterSpace;
+  StringRef PartData;
 
   using param_header_iterator = ViewArray<dxbc::RootParameterHeader>::iterator;
 
 public:
-  RootSignature() {}
+  RootSignature(StringRef PD) : PartData(PD) {}
 
   Error parse(StringRef Data);
   uint32_t getVersion() const { return Version; }
@@ -191,10 +190,6 @@ class RootSignature {
 
   llvm::Expected<RootParameterView>
   getParameter(const dxbc::RootParameterHeader &Header) const {
-    assert(ParameterSpaceOffset != 0 &&
-           "This should be initialized before reading parameters");
-    size_t CorrectOffset = Header.ParameterOffset - ParameterSpaceOffset;
-    StringRef Data;
     size_t DataSize;
 
     switch (Header.ParameterType) {
@@ -202,12 +197,15 @@ class RootSignature {
       DataSize = sizeof(dxbc::RootConstants);
       break;
     }
+    auto EndOfSectionByte = getNumStaticSamplers() == 0
+                                ? PartData.size()
+                                : getStaticSamplersOffset();
 
-    if (CorrectOffset + DataSize > ParameterSpace.size())
+    if (Header.ParameterOffset + DataSize > EndOfSectionByte)
       return parseFailed("Reading structure out of file bounds");
 
-    Data = ParameterSpace.substr(CorrectOffset, DataSize);
-    RootParameterView View = RootParameterView(Header, Data);
+    StringRef Buff = PartData.substr(Header.ParameterOffset, DataSize);
+    RootParameterView View = RootParameterView(Header, Buff);
     return View;
   }
 };
diff --git a/llvm/lib/Object/DXContainer.cpp b/llvm/lib/Object/DXContainer.cpp
index 2e014dd0c0c53..88e548267f97b 100644
--- a/llvm/lib/Object/DXContainer.cpp
+++ b/llvm/lib/Object/DXContainer.cpp
@@ -96,7 +96,7 @@ Error DXContainer::parseHash(StringRef Part) {
 Error DXContainer::parseRootSignature(StringRef Part) {
   if (RootSignature)
     return parseFailed("More than one RTS0 part is present in the file");
-  RootSignature = DirectX::RootSignature();
+  RootSignature = DirectX::RootSignature(Part);
   if (Error Err = RootSignature->parse(Part))
     return Err;
   return Error::success();
@@ -278,14 +278,6 @@ Error DirectX::RootSignature::parse(StringRef Data) {
   ParametersHeaders.Data = Data.substr(
       RootParametersOffset, NumParameters * sizeof(dxbc::RootParameterHeader));
 
-  ParameterSpaceOffset =
-      RootParametersOffset + NumParameters * sizeof(dxbc::RootParameterHeader);
-  size_t ParameterSpaceEnd =
-      (NumStaticSamplers == 0) ? Data.size() : StaticSamplersOffset;
-
-  ParameterSpace = Data.substr(ParameterSpaceOffset,
-                               ParameterSpaceEnd - ParameterSpaceOffset);
-
   return Error::success();
 }
 

>From 39d4b08570c55201cd6b57a6a520b01f7d04cd48 Mon Sep 17 00:00:00 2001
From: joaosaffran <joao.saffran at microsoft.com>
Date: Thu, 10 Apr 2025 21:23:22 +0000
Subject: [PATCH 6/9] addressing pr comments

---
 llvm/include/llvm/BinaryFormat/DXContainer.h  | 33 ++++++++++--
 .../llvm/MC/DXContainerRootSignature.h        |  6 +--
 llvm/include/llvm/Object/DXContainer.h        | 12 +----
 .../include/llvm/ObjectYAML/DXContainerYAML.h |  4 ++
 llvm/lib/MC/DXContainerRootSignature.cpp      |  4 +-
 llvm/lib/ObjectYAML/DXContainerEmitter.cpp    |  4 +-
 llvm/lib/ObjectYAML/DXContainerYAML.cpp       | 53 +++++++++++++------
 llvm/lib/Target/DirectX/DXILRootSignature.cpp | 18 ++++---
 llvm/unittests/Object/DXContainerTest.cpp     |  2 +-
 9 files changed, 88 insertions(+), 48 deletions(-)

diff --git a/llvm/include/llvm/BinaryFormat/DXContainer.h b/llvm/include/llvm/BinaryFormat/DXContainer.h
index e0a03d6d777a8..e23234603667b 100644
--- a/llvm/include/llvm/BinaryFormat/DXContainer.h
+++ b/llvm/include/llvm/BinaryFormat/DXContainer.h
@@ -14,6 +14,7 @@
 #define LLVM_BINARYFORMAT_DXCONTAINER_H
 
 #include "llvm/ADT/StringRef.h"
+#include "llvm/Support/Error.h"
 #include "llvm/Support/SwapByteOrder.h"
 #include "llvm/TargetParser/Triple.h"
 
@@ -158,19 +159,43 @@ enum class RootElementFlag : uint32_t {
 };
 
 #define ROOT_PARAMETER(Val, Enum) Enum = Val,
-enum class RootParameterType : uint32_t {
+enum RootParameterType : uint32_t {
 #include "DXContainerConstants.def"
 };
 
 ArrayRef<EnumEntry<RootParameterType>> getRootParameterTypes();
 
+#define ROOT_PARAMETER(Val, Enum)                                              \
+  case Val:                                                                    \
+    return dxbc::RootParameterType::Enum;
+inline llvm::Expected<dxbc::RootParameterType>
+safeParseParameterType(uint32_t V) {
+  switch (V) {
+#include "DXContainerConstants.def"
+  }
+  return createStringError(std::errc::invalid_argument,
+                           "Invalid value for parameter type");
+}
+
 #define SHADER_VISIBILITY(Val, Enum) Enum = Val,
-enum class ShaderVisibility : uint32_t {
+enum ShaderVisibility : uint32_t {
 #include "DXContainerConstants.def"
 };
 
 ArrayRef<EnumEntry<ShaderVisibility>> getShaderVisibility();
 
+#define SHADER_VISIBILITY(Val, Enum)                                           \
+  case Val:                                                                    \
+    return dxbc::ShaderVisibility::Enum;
+inline llvm::Expected<dxbc::ShaderVisibility>
+safeParseShaderVisibility(uint32_t V) {
+  switch (V) {
+#include "DXContainerConstants.def"
+  }
+  return createStringError(std::errc::invalid_argument,
+                           "Invalid value for parameter type");
+}
+
 PartType parsePartType(StringRef S);
 
 struct VertexPSVInfo {
@@ -575,8 +600,8 @@ struct RootConstants {
 };
 
 struct RootParameterHeader {
-  RootParameterType ParameterType;
-  ShaderVisibility ShaderVisibility;
+  uint32_t ParameterType;
+  uint32_t ShaderVisibility;
   uint32_t ParameterOffset;
 
   void swapBytes() {
diff --git a/llvm/include/llvm/MC/DXContainerRootSignature.h b/llvm/include/llvm/MC/DXContainerRootSignature.h
index 0ad7b21981415..d0f5e2af861d8 100644
--- a/llvm/include/llvm/MC/DXContainerRootSignature.h
+++ b/llvm/include/llvm/MC/DXContainerRootSignature.h
@@ -23,11 +23,9 @@ struct RootParameter {
 };
 struct RootSignatureDesc {
 
-  dxbc::RootSignatureHeader Header;
+  uint32_t Version = 2U;
+  uint32_t Flags = 0U;
   SmallVector<mcdxbc::RootParameter> Parameters;
-  RootSignatureDesc()
-      : Header(dxbc::RootSignatureHeader{
-            2, 0, sizeof(dxbc::RootSignatureHeader), 0, 0, 0}) {}
 
   void write(raw_ostream &OS) const;
 
diff --git a/llvm/include/llvm/Object/DXContainer.h b/llvm/include/llvm/Object/DXContainer.h
index eeee443cb9460..656ef864c9473 100644
--- a/llvm/include/llvm/Object/DXContainer.h
+++ b/llvm/include/llvm/Object/DXContainer.h
@@ -117,16 +117,6 @@ template <typename T> struct ViewArray {
 };
 
 namespace DirectX {
-
-struct RootParameter {
-  dxbc::RootParameterHeader Header;
-  union {
-    dxbc::RootConstants Constants;
-  };
-
-  RootParameter() = default;
-};
-
 struct RootParameterView {
   const dxbc::RootParameterHeader &Header;
   StringRef ParamData;
@@ -183,7 +173,7 @@ class RootSignature {
   uint32_t getRootParametersOffset() const { return RootParametersOffset; }
   uint32_t getNumStaticSamplers() const { return NumStaticSamplers; }
   uint32_t getStaticSamplersOffset() const { return StaticSamplersOffset; }
-  llvm::iterator_range<param_header_iterator> param_header() const {
+  llvm::iterator_range<param_header_iterator> param_headers() const {
     return llvm::make_range(ParametersHeaders.begin(), ParametersHeaders.end());
   }
   uint32_t getFlags() const { return Flags; }
diff --git a/llvm/include/llvm/ObjectYAML/DXContainerYAML.h b/llvm/include/llvm/ObjectYAML/DXContainerYAML.h
index 9d453d89bf73d..baafbdc6c9151 100644
--- a/llvm/include/llvm/ObjectYAML/DXContainerYAML.h
+++ b/llvm/include/llvm/ObjectYAML/DXContainerYAML.h
@@ -103,6 +103,10 @@ struct RootSignatureYamlDesc {
 
   uint32_t getEncodedFlags();
 
+  iterator_range<RootParameterYamlDesc *> params() {
+    return make_range(Parameters.begin(), Parameters.end());
+  }
+
 #include "llvm/BinaryFormat/DXContainerConstants.def"
 };
 
diff --git a/llvm/lib/MC/DXContainerRootSignature.cpp b/llvm/lib/MC/DXContainerRootSignature.cpp
index a846e4572c449..63de60acc9bc2 100644
--- a/llvm/lib/MC/DXContainerRootSignature.cpp
+++ b/llvm/lib/MC/DXContainerRootSignature.cpp
@@ -51,13 +51,13 @@ void RootSignatureDesc::write(raw_ostream &OS) const {
   const uint32_t StaticSamplerOffset = 0u;
   const uint32_t NumStaticSamplers = 0u;
 
-  support::endian::write(BOS, Header.Version, llvm::endianness::little);
+  support::endian::write(BOS, Version, llvm::endianness::little);
   support::endian::write(BOS, NumParameters, llvm::endianness::little);
   support::endian::write(BOS, (uint32_t)sizeof(dxbc::RootSignatureHeader),
                          llvm::endianness::little);
   support::endian::write(BOS, StaticSamplerOffset, llvm::endianness::little);
   support::endian::write(BOS, NumStaticSamplers, llvm::endianness::little);
-  support::endian::write(BOS, Header.Flags, llvm::endianness::little);
+  support::endian::write(BOS, Flags, llvm::endianness::little);
 
   SmallVector<uint32_t> ParamsOffsets;
   for (const auto &P : Parameters) {
diff --git a/llvm/lib/ObjectYAML/DXContainerEmitter.cpp b/llvm/lib/ObjectYAML/DXContainerEmitter.cpp
index 2092dd9f1e6e5..1342629df84f3 100644
--- a/llvm/lib/ObjectYAML/DXContainerEmitter.cpp
+++ b/llvm/lib/ObjectYAML/DXContainerEmitter.cpp
@@ -267,8 +267,8 @@ void DXContainerWriter::writeParts(raw_ostream &OS) {
         continue;
 
       mcdxbc::RootSignatureDesc RS;
-      RS.Header.Flags = P.RootSignature->getEncodedFlags();
-      RS.Header.Version = P.RootSignature->Version;
+      RS.Flags = P.RootSignature->getEncodedFlags();
+      RS.Version = P.RootSignature->Version;
       for (const auto &Param : P.RootSignature->Parameters) {
         mcdxbc::RootParameter NewParam;
         NewParam.Header = dxbc::RootParameterHeader{
diff --git a/llvm/lib/ObjectYAML/DXContainerYAML.cpp b/llvm/lib/ObjectYAML/DXContainerYAML.cpp
index e8578df8aa509..0791cccd244b2 100644
--- a/llvm/lib/ObjectYAML/DXContainerYAML.cpp
+++ b/llvm/lib/ObjectYAML/DXContainerYAML.cpp
@@ -35,27 +35,48 @@ DXContainerYAML::RootSignatureYamlDesc::RootSignatureYamlDesc(
       NumStaticSamplers(Data.getNumStaticSamplers()),
       StaticSamplersOffset(Data.getStaticSamplersOffset()) {
   uint32_t Flags = Data.getFlags();
-  for (const auto &PH : Data.param_header()) {
+  for (const auto &PH : Data.param_headers()) {
 
     RootParameterYamlDesc NewP;
     NewP.Offset = PH.ParameterOffset;
-    NewP.Type = PH.ParameterType;
-    NewP.Visibility = PH.ShaderVisibility;
 
-    llvm::Expected<object::DirectX::RootParameterView> ParamView =
+    llvm::Expected<dxbc::RootParameterType> TypeOrErr =
+        dxbc::safeParseParameterType(PH.ParameterType);
+    if (Error E = TypeOrErr.takeError()) {
+      llvm::errs() << "Error: " << E << "\n";
+      continue;
+    }
+
+    NewP.Type = TypeOrErr.get();
+
+    llvm::Expected<dxbc::ShaderVisibility> VisibilityOrErr =
+        dxbc::safeParseShaderVisibility(PH.ShaderVisibility);
+    if (Error E = VisibilityOrErr.takeError()) {
+      llvm::errs() << "Error: " << E << "\n";
+      continue;
+    }
+    NewP.Visibility = VisibilityOrErr.get();
+
+    llvm::Expected<object::DirectX::RootParameterView> ParamViewOrErr =
         Data.getParameter(PH);
-    if (!ParamView)
-      llvm::errs() << "Error: " << ParamView.takeError() << "\n";
-    auto PV = *ParamView;
-
-    if (auto *RCV = dyn_cast<object::DirectX::RootConstantView>(&PV)) {
-      auto Constants = RCV->read();
-      if (!Constants)
-        llvm::errs() << "Error: " << Constants.takeError() << "\n";
-
-      NewP.Constants.Num32BitValues = Constants->Num32BitValues;
-      NewP.Constants.ShaderRegister = Constants->ShaderRegister;
-      NewP.Constants.RegisterSpace = Constants->RegisterSpace;
+    if (Error E = ParamViewOrErr.takeError()) {
+      llvm::errs() << "Error: " << E << "\n";
+      continue;
+    }
+    object::DirectX::RootParameterView ParamView = ParamViewOrErr.get();
+
+    if (auto *RCV = dyn_cast<object::DirectX::RootConstantView>(&ParamView)) {
+      llvm::Expected<dxbc::RootConstants> ConstantsOrErr = RCV->read();
+      if (Error E = ConstantsOrErr.takeError()) {
+        llvm::errs() << "Error: " << E << "\n";
+        continue;
+      }
+
+      auto Constants = *ConstantsOrErr;
+
+      NewP.Constants.Num32BitValues = Constants.Num32BitValues;
+      NewP.Constants.ShaderRegister = Constants.ShaderRegister;
+      NewP.Constants.RegisterSpace = Constants.RegisterSpace;
     }
     Parameters.push_back(NewP);
   }
diff --git a/llvm/lib/Target/DirectX/DXILRootSignature.cpp b/llvm/lib/Target/DirectX/DXILRootSignature.cpp
index 412ab7765a7ae..3ba0535e0114b 100644
--- a/llvm/lib/Target/DirectX/DXILRootSignature.cpp
+++ b/llvm/lib/Target/DirectX/DXILRootSignature.cpp
@@ -47,7 +47,7 @@ static bool parseRootFlags(LLVMContext *Ctx, mcdxbc::RootSignatureDesc &RSD,
     return reportError(Ctx, "Invalid format for RootFlag Element");
 
   auto *Flag = mdconst::extract<ConstantInt>(RootFlagNode->getOperand(1));
-  RSD.Header.Flags = Flag->getZExtValue();
+  RSD.Flags = Flag->getZExtValue();
 
   return false;
 }
@@ -95,7 +95,7 @@ static bool parse(LLVMContext *Ctx, mcdxbc::RootSignatureDesc &RSD,
 static bool verifyRootFlag(uint32_t Flags) { return (Flags & ~0xfff) == 0; }
 
 static bool validate(LLVMContext *Ctx, const mcdxbc::RootSignatureDesc &RSD) {
-  if (!verifyRootFlag(RSD.Header.Flags)) {
+  if (!verifyRootFlag(RSD.Flags)) {
     return reportError(Ctx, "Invalid Root Signature flag value");
   }
   return false;
@@ -191,6 +191,8 @@ PreservedAnalyses RootSignatureAnalysisPrinter::run(Module &M,
 
   SmallDenseMap<const Function *, mcdxbc::RootSignatureDesc> &RSDMap =
       AM.getResult<RootSignatureAnalysis>(M);
+
+  const size_t RSHSize = sizeof(dxbc::RootSignatureHeader);
   OS << "Root Signature Definitions"
      << "\n";
   uint8_t Space = 0;
@@ -203,14 +205,14 @@ PreservedAnalyses RootSignatureAnalysisPrinter::run(Module &M,
 
     // start root signature header
     Space++;
-    OS << indent(Space) << "Flags: " << format_hex(RS.Header.Flags, 8) << ":\n";
-    OS << indent(Space) << "Version: " << RS.Header.Version << ":\n";
+    OS << indent(Space) << "Flags: " << format_hex(RS.Flags, 8) << ":\n";
+    OS << indent(Space) << "Version: " << RS.Version << ":\n";
     OS << indent(Space) << "NumParameters: " << RS.Parameters.size() << ":\n";
-    OS << indent(Space) << "RootParametersOffset: " << sizeof(RS.Header)
-       << ":\n";
+    OS << indent(Space) << "RootParametersOffset: " << RSHSize << ":\n";
     OS << indent(Space) << "NumStaticSamplers: " << 0 << ":\n";
-    OS << indent(Space) << "StaticSamplersOffset: "
-       << sizeof(RS.Header) + RS.Parameters.size_in_bytes() << ":\n";
+    OS << indent(Space)
+       << "StaticSamplersOffset: " << RSHSize + RS.Parameters.size_in_bytes()
+       << ":\n";
     Space--;
     // end root signature header
   }
diff --git a/llvm/unittests/Object/DXContainerTest.cpp b/llvm/unittests/Object/DXContainerTest.cpp
index b2ff9ad218de0..bf32c07951520 100644
--- a/llvm/unittests/Object/DXContainerTest.cpp
+++ b/llvm/unittests/Object/DXContainerTest.cpp
@@ -890,7 +890,7 @@ TEST(RootSignature, ParseRootConstant) {
     ASSERT_EQ(RS.getStaticSamplersOffset(), 44u);
     ASSERT_EQ(RS.getFlags(), 17u);
 
-    auto RootParam = *RS.param_header().begin();
+    auto RootParam = *RS.param_headers().begin();
     ASSERT_EQ((unsigned)RootParam.ParameterType, 1u);
     ASSERT_EQ((unsigned)RootParam.ShaderVisibility, 2u);
     auto ParamView = RS.getParameter(RootParam);

>From 7343d953a60e9f07fdb1c54c988690cfbff51bae Mon Sep 17 00:00:00 2001
From: joaosaffran <joao.saffran at microsoft.com>
Date: Fri, 11 Apr 2025 19:18:29 +0000
Subject: [PATCH 7/9] adding more tests

---
 .../include/llvm/ObjectYAML/DXContainerYAML.h |  8 +++--
 llvm/lib/ObjectYAML/DXContainerYAML.cpp       | 34 +++++++++++--------
 llvm/tools/obj2yaml/dxcontainer2yaml.cpp      |  9 +++--
 llvm/unittests/Object/DXContainerTest.cpp     | 23 +++++++++++++
 .../ObjectYAML/DXContainerYAMLTest.cpp        |  1 +
 5 files changed, 55 insertions(+), 20 deletions(-)

diff --git a/llvm/include/llvm/ObjectYAML/DXContainerYAML.h b/llvm/include/llvm/ObjectYAML/DXContainerYAML.h
index baafbdc6c9151..4718b5fd61adf 100644
--- a/llvm/include/llvm/ObjectYAML/DXContainerYAML.h
+++ b/llvm/include/llvm/ObjectYAML/DXContainerYAML.h
@@ -92,9 +92,6 @@ struct RootParameterYamlDesc {
 };
 
 struct RootSignatureYamlDesc {
-  RootSignatureYamlDesc() = default;
-  RootSignatureYamlDesc(const object::DirectX::RootSignature &Data);
-
   uint32_t Version;
   uint32_t NumStaticSamplers;
   uint32_t StaticSamplersOffset;
@@ -107,7 +104,12 @@ struct RootSignatureYamlDesc {
     return make_range(Parameters.begin(), Parameters.end());
   }
 
+  static llvm::Expected<DXContainerYAML::RootSignatureYamlDesc>
+  create(const object::DirectX::RootSignature &Data);
+
 #include "llvm/BinaryFormat/DXContainerConstants.def"
+
+  RootSignatureYamlDesc() = default;
 };
 
 using ResourceFlags = dxbc::PSV::ResourceFlags;
diff --git a/llvm/lib/ObjectYAML/DXContainerYAML.cpp b/llvm/lib/ObjectYAML/DXContainerYAML.cpp
index 0791cccd244b2..e80e4c3543da4 100644
--- a/llvm/lib/ObjectYAML/DXContainerYAML.cpp
+++ b/llvm/lib/ObjectYAML/DXContainerYAML.cpp
@@ -14,7 +14,9 @@
 #include "llvm/ObjectYAML/DXContainerYAML.h"
 #include "llvm/ADT/ScopeExit.h"
 #include "llvm/BinaryFormat/DXContainer.h"
+#include "llvm/Support/Error.h"
 #include "llvm/Support/ScopedPrinter.h"
+#include <utility>
 
 namespace llvm {
 
@@ -29,11 +31,16 @@ DXContainerYAML::ShaderFeatureFlags::ShaderFeatureFlags(uint64_t FlagData) {
 #include "llvm/BinaryFormat/DXContainerConstants.def"
 }
 
-DXContainerYAML::RootSignatureYamlDesc::RootSignatureYamlDesc(
-    const object::DirectX::RootSignature &Data)
-    : Version(Data.getVersion()),
-      NumStaticSamplers(Data.getNumStaticSamplers()),
-      StaticSamplersOffset(Data.getStaticSamplersOffset()) {
+llvm::Expected<DXContainerYAML::RootSignatureYamlDesc>
+DXContainerYAML::RootSignatureYamlDesc::create(
+    const object::DirectX::RootSignature &Data) {
+
+  RootSignatureYamlDesc RootSigDesc;
+
+  RootSigDesc.Version = Data.getVersion();
+  RootSigDesc.NumStaticSamplers = Data.getNumStaticSamplers();
+  RootSigDesc.StaticSamplersOffset = Data.getStaticSamplersOffset();
+
   uint32_t Flags = Data.getFlags();
   for (const auto &PH : Data.param_headers()) {
 
@@ -43,8 +50,7 @@ DXContainerYAML::RootSignatureYamlDesc::RootSignatureYamlDesc(
     llvm::Expected<dxbc::RootParameterType> TypeOrErr =
         dxbc::safeParseParameterType(PH.ParameterType);
     if (Error E = TypeOrErr.takeError()) {
-      llvm::errs() << "Error: " << E << "\n";
-      continue;
+      return std::move(E);
     }
 
     NewP.Type = TypeOrErr.get();
@@ -52,24 +58,21 @@ DXContainerYAML::RootSignatureYamlDesc::RootSignatureYamlDesc(
     llvm::Expected<dxbc::ShaderVisibility> VisibilityOrErr =
         dxbc::safeParseShaderVisibility(PH.ShaderVisibility);
     if (Error E = VisibilityOrErr.takeError()) {
-      llvm::errs() << "Error: " << E << "\n";
-      continue;
+      return std::move(E);
     }
     NewP.Visibility = VisibilityOrErr.get();
 
     llvm::Expected<object::DirectX::RootParameterView> ParamViewOrErr =
         Data.getParameter(PH);
     if (Error E = ParamViewOrErr.takeError()) {
-      llvm::errs() << "Error: " << E << "\n";
-      continue;
+      return std::move(E);
     }
     object::DirectX::RootParameterView ParamView = ParamViewOrErr.get();
 
     if (auto *RCV = dyn_cast<object::DirectX::RootConstantView>(&ParamView)) {
       llvm::Expected<dxbc::RootConstants> ConstantsOrErr = RCV->read();
       if (Error E = ConstantsOrErr.takeError()) {
-        llvm::errs() << "Error: " << E << "\n";
-        continue;
+        return std::move(E);
       }
 
       auto Constants = *ConstantsOrErr;
@@ -78,11 +81,12 @@ DXContainerYAML::RootSignatureYamlDesc::RootSignatureYamlDesc(
       NewP.Constants.ShaderRegister = Constants.ShaderRegister;
       NewP.Constants.RegisterSpace = Constants.RegisterSpace;
     }
-    Parameters.push_back(NewP);
+    RootSigDesc.Parameters.push_back(NewP);
   }
 #define ROOT_ELEMENT_FLAG(Num, Val)                                            \
-  Val = (Flags & (uint32_t)dxbc::RootElementFlag::Val) > 0;
+  RootSigDesc.Val = (Flags & (uint32_t)dxbc::RootElementFlag::Val) > 0;
 #include "llvm/BinaryFormat/DXContainerConstants.def"
+  return RootSigDesc;
 }
 
 uint32_t DXContainerYAML::RootSignatureYamlDesc::getEncodedFlags() {
diff --git a/llvm/tools/obj2yaml/dxcontainer2yaml.cpp b/llvm/tools/obj2yaml/dxcontainer2yaml.cpp
index f3ef1b6a27bcf..c727595406767 100644
--- a/llvm/tools/obj2yaml/dxcontainer2yaml.cpp
+++ b/llvm/tools/obj2yaml/dxcontainer2yaml.cpp
@@ -155,8 +155,13 @@ dumpDXContainer(MemoryBufferRef Source) {
       break;
     case dxbc::PartType::RTS0:
       std::optional<DirectX::RootSignature> RS = Container.getRootSignature();
-      if (RS.has_value())
-        NewPart.RootSignature = DXContainerYAML::RootSignatureYamlDesc(*RS);
+      if (RS.has_value()) {
+        auto RootSigDescOrErr =
+            DXContainerYAML::RootSignatureYamlDesc::create(*RS);
+        if (Error E = RootSigDescOrErr.takeError())
+          return std::move(E);
+        NewPart.RootSignature = RootSigDescOrErr.get();
+      }
       break;
     }
   }
diff --git a/llvm/unittests/Object/DXContainerTest.cpp b/llvm/unittests/Object/DXContainerTest.cpp
index bf32c07951520..59591435480bc 100644
--- a/llvm/unittests/Object/DXContainerTest.cpp
+++ b/llvm/unittests/Object/DXContainerTest.cpp
@@ -7,12 +7,15 @@
 //===----------------------------------------------------------------------===//
 
 #include "llvm/Object/DXContainer.h"
+#include "../../tools/obj2yaml/dxcontainer2yaml.cpp"
+#include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/BinaryFormat/Magic.h"
 #include "llvm/ObjectYAML/DXContainerYAML.h"
 #include "llvm/ObjectYAML/yaml2obj.h"
 #include "llvm/Support/MemoryBufferRef.h"
 #include "llvm/Testing/Support/Error.h"
+#include "gmock/gmock.h"
 #include "gtest/gtest.h"
 
 using namespace llvm;
@@ -907,4 +910,24 @@ TEST(RootSignature, ParseRootConstant) {
     ASSERT_EQ(Constants->RegisterSpace, 14u);
     ASSERT_EQ(Constants->Num32BitValues, 16u);
   }
+  {
+    uint8_t Buffer[] = {
+        0x44, 0x58, 0x42, 0x43, 0x32, 0x9a, 0x53, 0xd8, 0xec, 0xbe, 0x35, 0x6f,
+        0x05, 0x39, 0xe1, 0xfe, 0x31, 0x20, 0xf0, 0xc1, 0x01, 0x00, 0x00, 0x00,
+        0x85, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x24, 0x00, 0x00, 0x00,
+        0x52, 0x54, 0x53, 0x30, 0x59, 0x00, 0x00, 0x00, 0x02, 0x00, 0x00, 0x00,
+        0x01, 0x00, 0x00, 0x00, 0x18, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+        0x2c, 0x00, 0x00, 0x00, 0x11, 0x00, 0x00, 0x00, 0xFF, 0x00, 0x00, 0x00,
+        0x02, 0x00, 0x00, 0x00, 0x24, 0x00, 0x00, 0x00, 0x0f, 0x00, 0x00, 0x00,
+        0x0e, 0x00, 0x00, 0x00, 0x10, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+        0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+        0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+        0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+        0x00};
+
+    SmallString<256> Storage;
+    raw_svector_ostream OS(Storage);
+    EXPECT_THAT_ERROR(dxcontainer2yaml(OS, getMemoryBuffer<133>(Buffer)),
+                      FailedWithMessage("Invalid value for parameter type"));
+  }
 }
diff --git a/llvm/unittests/ObjectYAML/DXContainerYAMLTest.cpp b/llvm/unittests/ObjectYAML/DXContainerYAMLTest.cpp
index 3e5b79be2ccff..73b2555dfd9a9 100644
--- a/llvm/unittests/ObjectYAML/DXContainerYAMLTest.cpp
+++ b/llvm/unittests/ObjectYAML/DXContainerYAMLTest.cpp
@@ -6,6 +6,7 @@
 //
 //===----------------------------------------------------------------------===//
 
+#include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/Twine.h"
 #include "llvm/ObjectYAML/ObjectYAML.h"

>From 69869451be4e9e9ab4f6bdd9468ab02728670f29 Mon Sep 17 00:00:00 2001
From: joaosaffran <joao.saffran at microsoft.com>
Date: Fri, 11 Apr 2025 19:43:26 +0000
Subject: [PATCH 8/9] adding more tests and addressing comments

---
 llvm/include/llvm/BinaryFormat/DXContainer.h  | 16 +++++--------
 .../include/llvm/ObjectYAML/DXContainerYAML.h |  4 ++--
 llvm/lib/ObjectYAML/DXContainerYAML.cpp       | 23 ++++++++-----------
 llvm/unittests/Object/DXContainerTest.cpp     | 22 ++++++++++++++++--
 .../ObjectYAML/DXContainerYAMLTest.cpp        |  1 -
 5 files changed, 38 insertions(+), 28 deletions(-)

diff --git a/llvm/include/llvm/BinaryFormat/DXContainer.h b/llvm/include/llvm/BinaryFormat/DXContainer.h
index e23234603667b..56df554b56e2e 100644
--- a/llvm/include/llvm/BinaryFormat/DXContainer.h
+++ b/llvm/include/llvm/BinaryFormat/DXContainer.h
@@ -167,14 +167,12 @@ ArrayRef<EnumEntry<RootParameterType>> getRootParameterTypes();
 
 #define ROOT_PARAMETER(Val, Enum)                                              \
   case Val:                                                                    \
-    return dxbc::RootParameterType::Enum;
-inline llvm::Expected<dxbc::RootParameterType>
-safeParseParameterType(uint32_t V) {
+    return true;
+inline bool isValidParameterType(uint32_t V) {
   switch (V) {
 #include "DXContainerConstants.def"
   }
-  return createStringError(std::errc::invalid_argument,
-                           "Invalid value for parameter type");
+  return false;
 }
 
 #define SHADER_VISIBILITY(Val, Enum) Enum = Val,
@@ -186,14 +184,12 @@ ArrayRef<EnumEntry<ShaderVisibility>> getShaderVisibility();
 
 #define SHADER_VISIBILITY(Val, Enum)                                           \
   case Val:                                                                    \
-    return dxbc::ShaderVisibility::Enum;
-inline llvm::Expected<dxbc::ShaderVisibility>
-safeParseShaderVisibility(uint32_t V) {
+    return true;
+inline bool isValidShaderVisibility(uint32_t V) {
   switch (V) {
 #include "DXContainerConstants.def"
   }
-  return createStringError(std::errc::invalid_argument,
-                           "Invalid value for parameter type");
+  return false;
 }
 
 PartType parsePartType(StringRef S);
diff --git a/llvm/include/llvm/ObjectYAML/DXContainerYAML.h b/llvm/include/llvm/ObjectYAML/DXContainerYAML.h
index 4718b5fd61adf..c57a879865011 100644
--- a/llvm/include/llvm/ObjectYAML/DXContainerYAML.h
+++ b/llvm/include/llvm/ObjectYAML/DXContainerYAML.h
@@ -92,6 +92,8 @@ struct RootParameterYamlDesc {
 };
 
 struct RootSignatureYamlDesc {
+  RootSignatureYamlDesc() = default;
+
   uint32_t Version;
   uint32_t NumStaticSamplers;
   uint32_t StaticSamplersOffset;
@@ -108,8 +110,6 @@ struct RootSignatureYamlDesc {
   create(const object::DirectX::RootSignature &Data);
 
 #include "llvm/BinaryFormat/DXContainerConstants.def"
-
-  RootSignatureYamlDesc() = default;
 };
 
 using ResourceFlags = dxbc::PSV::ResourceFlags;
diff --git a/llvm/lib/ObjectYAML/DXContainerYAML.cpp b/llvm/lib/ObjectYAML/DXContainerYAML.cpp
index e80e4c3543da4..7d43e6c7f66b8 100644
--- a/llvm/lib/ObjectYAML/DXContainerYAML.cpp
+++ b/llvm/lib/ObjectYAML/DXContainerYAML.cpp
@@ -16,7 +16,7 @@
 #include "llvm/BinaryFormat/DXContainer.h"
 #include "llvm/Support/Error.h"
 #include "llvm/Support/ScopedPrinter.h"
-#include <utility>
+#include <system_error>
 
 namespace llvm {
 
@@ -47,20 +47,17 @@ DXContainerYAML::RootSignatureYamlDesc::create(
     RootParameterYamlDesc NewP;
     NewP.Offset = PH.ParameterOffset;
 
-    llvm::Expected<dxbc::RootParameterType> TypeOrErr =
-        dxbc::safeParseParameterType(PH.ParameterType);
-    if (Error E = TypeOrErr.takeError()) {
-      return std::move(E);
-    }
+    if (!dxbc::isValidParameterType(PH.ParameterType))
+      return createStringError(std::errc::invalid_argument,
+                               "Invalid value for parameter type");
 
-    NewP.Type = TypeOrErr.get();
+    NewP.Type = (dxbc::RootParameterType)PH.ParameterType;
 
-    llvm::Expected<dxbc::ShaderVisibility> VisibilityOrErr =
-        dxbc::safeParseShaderVisibility(PH.ShaderVisibility);
-    if (Error E = VisibilityOrErr.takeError()) {
-      return std::move(E);
-    }
-    NewP.Visibility = VisibilityOrErr.get();
+    if (!dxbc::isValidShaderVisibility(PH.ShaderVisibility))
+      return createStringError(std::errc::invalid_argument,
+                               "Invalid value for shader visibility");
+
+    NewP.Visibility = (dxbc::ShaderVisibility)PH.ShaderVisibility;
 
     llvm::Expected<object::DirectX::RootParameterView> ParamViewOrErr =
         Data.getParameter(PH);
diff --git a/llvm/unittests/Object/DXContainerTest.cpp b/llvm/unittests/Object/DXContainerTest.cpp
index 59591435480bc..e8c299f4d5c08 100644
--- a/llvm/unittests/Object/DXContainerTest.cpp
+++ b/llvm/unittests/Object/DXContainerTest.cpp
@@ -8,14 +8,12 @@
 
 #include "llvm/Object/DXContainer.h"
 #include "../../tools/obj2yaml/dxcontainer2yaml.cpp"
-#include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/BinaryFormat/Magic.h"
 #include "llvm/ObjectYAML/DXContainerYAML.h"
 #include "llvm/ObjectYAML/yaml2obj.h"
 #include "llvm/Support/MemoryBufferRef.h"
 #include "llvm/Testing/Support/Error.h"
-#include "gmock/gmock.h"
 #include "gtest/gtest.h"
 
 using namespace llvm;
@@ -930,4 +928,24 @@ TEST(RootSignature, ParseRootConstant) {
     EXPECT_THAT_ERROR(dxcontainer2yaml(OS, getMemoryBuffer<133>(Buffer)),
                       FailedWithMessage("Invalid value for parameter type"));
   }
+  {
+    uint8_t Buffer[] = {
+        0x44, 0x58, 0x42, 0x43, 0x32, 0x9a, 0x53, 0xd8, 0xec, 0xbe, 0x35, 0x6f,
+        0x05, 0x39, 0xe1, 0xfe, 0x31, 0x20, 0xf0, 0xc1, 0x01, 0x00, 0x00, 0x00,
+        0x85, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x24, 0x00, 0x00, 0x00,
+        0x52, 0x54, 0x53, 0x30, 0x59, 0x00, 0x00, 0x00, 0x02, 0x00, 0x00, 0x00,
+        0x01, 0x00, 0x00, 0x00, 0x18, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+        0x2c, 0x00, 0x00, 0x00, 0x11, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00,
+        0xFF, 0x00, 0x00, 0x00, 0x24, 0x00, 0x00, 0x00, 0x0f, 0x00, 0x00, 0x00,
+        0x0e, 0x00, 0x00, 0x00, 0x10, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+        0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+        0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+        0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+        0x00};
+
+    SmallString<256> Storage;
+    raw_svector_ostream OS(Storage);
+    EXPECT_THAT_ERROR(dxcontainer2yaml(OS, getMemoryBuffer<133>(Buffer)),
+                      FailedWithMessage("Invalid value for shader visibility"));
+  }
 }
diff --git a/llvm/unittests/ObjectYAML/DXContainerYAMLTest.cpp b/llvm/unittests/ObjectYAML/DXContainerYAMLTest.cpp
index 73b2555dfd9a9..3e5b79be2ccff 100644
--- a/llvm/unittests/ObjectYAML/DXContainerYAMLTest.cpp
+++ b/llvm/unittests/ObjectYAML/DXContainerYAMLTest.cpp
@@ -6,7 +6,6 @@
 //
 //===----------------------------------------------------------------------===//
 
-#include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/Twine.h"
 #include "llvm/ObjectYAML/ObjectYAML.h"

>From c0ac522ce0000f58092c99b66bc94150aed70122 Mon Sep 17 00:00:00 2001
From: joaosaffran <joao.saffran at microsoft.com>
Date: Mon, 14 Apr 2025 18:39:35 +0000
Subject: [PATCH 9/9] adding tests and changing parameter type

---
 llvm/include/llvm/Object/DXContainer.h        |  3 +-
 .../include/llvm/ObjectYAML/DXContainerYAML.h |  8 ++--
 llvm/lib/Object/DXContainer.cpp               | 13 +++---
 llvm/lib/ObjectYAML/DXContainerYAML.cpp       | 20 +++------
 .../ContainerData/RootSignature-Flags.ll      |  2 +
 .../DXContainer/RootSignature-Flags.yaml      |  4 ++
 .../RootSignature-InvalidType.yaml            | 29 +++++++++++++
 .../RootSignature-InvalidVisibility.yaml      | 33 +++++++++++++++
 .../RootSignature-MultipleParameters.yaml     | 20 +++++----
 llvm/unittests/Object/DXContainerTest.cpp     | 41 -------------------
 .../ObjectYAML/DXContainerYAMLTest.cpp        |  8 +++-
 11 files changed, 104 insertions(+), 77 deletions(-)
 create mode 100644 llvm/test/ObjectYAML/DXContainer/RootSignature-InvalidType.yaml
 create mode 100644 llvm/test/ObjectYAML/DXContainer/RootSignature-InvalidVisibility.yaml

diff --git a/llvm/include/llvm/Object/DXContainer.h b/llvm/include/llvm/Object/DXContainer.h
index 656ef864c9473..561e4c1d1c634 100644
--- a/llvm/include/llvm/Object/DXContainer.h
+++ b/llvm/include/llvm/Object/DXContainer.h
@@ -167,12 +167,13 @@ class RootSignature {
 public:
   RootSignature(StringRef PD) : PartData(PD) {}
 
-  Error parse(StringRef Data);
+  Error parse();
   uint32_t getVersion() const { return Version; }
   uint32_t getNumParameters() const { return NumParameters; }
   uint32_t getRootParametersOffset() const { return RootParametersOffset; }
   uint32_t getNumStaticSamplers() const { return NumStaticSamplers; }
   uint32_t getStaticSamplersOffset() const { return StaticSamplersOffset; }
+  uint32_t getNumRootParameters() const { return ParametersHeaders.size(); }
   llvm::iterator_range<param_header_iterator> param_headers() const {
     return llvm::make_range(ParametersHeaders.begin(), ParametersHeaders.end());
   }
diff --git a/llvm/include/llvm/ObjectYAML/DXContainerYAML.h b/llvm/include/llvm/ObjectYAML/DXContainerYAML.h
index c57a879865011..393bba9c79bf8 100644
--- a/llvm/include/llvm/ObjectYAML/DXContainerYAML.h
+++ b/llvm/include/llvm/ObjectYAML/DXContainerYAML.h
@@ -82,8 +82,8 @@ struct RootConstantsYaml {
 };
 
 struct RootParameterYamlDesc {
-  dxbc::RootParameterType Type;
-  dxbc::ShaderVisibility Visibility;
+  uint32_t Type;
+  uint32_t Visibility;
   uint32_t Offset;
 
   union {
@@ -95,6 +95,8 @@ struct RootSignatureYamlDesc {
   RootSignatureYamlDesc() = default;
 
   uint32_t Version;
+  uint32_t NumRootParameters;
+  uint32_t RootParametersOffset;
   uint32_t NumStaticSamplers;
   uint32_t StaticSamplersOffset;
 
@@ -224,8 +226,6 @@ LLVM_YAML_DECLARE_ENUM_TRAITS(llvm::dxbc::PSV::ResourceKind)
 LLVM_YAML_DECLARE_ENUM_TRAITS(llvm::dxbc::D3DSystemValue)
 LLVM_YAML_DECLARE_ENUM_TRAITS(llvm::dxbc::SigComponentType)
 LLVM_YAML_DECLARE_ENUM_TRAITS(llvm::dxbc::SigMinPrecision)
-LLVM_YAML_DECLARE_ENUM_TRAITS(llvm::dxbc::RootParameterType)
-LLVM_YAML_DECLARE_ENUM_TRAITS(llvm::dxbc::ShaderVisibility)
 
 namespace llvm {
 
diff --git a/llvm/lib/Object/DXContainer.cpp b/llvm/lib/Object/DXContainer.cpp
index 88e548267f97b..77303edce1a41 100644
--- a/llvm/lib/Object/DXContainer.cpp
+++ b/llvm/lib/Object/DXContainer.cpp
@@ -97,7 +97,7 @@ Error DXContainer::parseRootSignature(StringRef Part) {
   if (RootSignature)
     return parseFailed("More than one RTS0 part is present in the file");
   RootSignature = DirectX::RootSignature(Part);
-  if (Error Err = RootSignature->parse(Part))
+  if (Error Err = RootSignature->parse())
     return Err;
   return Error::success();
 }
@@ -242,12 +242,11 @@ void DXContainer::PartIterator::updateIteratorImpl(const uint32_t Offset) {
   IteratorState.Offset = Offset;
 }
 
-Error DirectX::RootSignature::parse(StringRef Data) {
-  const char *Begin = Data.begin();
-  const char *Current = Data.begin();
+Error DirectX::RootSignature::parse() {
+  const char *Current = PartData.begin();
 
   // Root Signature headers expects 6 integers to be present.
-  if (Data.size() < 6 * sizeof(uint32_t))
+  if (PartData.size() < 6 * sizeof(uint32_t))
     return parseFailed(
         "Invalid root signature, insufficient space for header.");
 
@@ -273,9 +272,9 @@ Error DirectX::RootSignature::parse(StringRef Data) {
   Flags = support::endian::read<uint32_t, llvm::endianness::little>(Current);
   Current += sizeof(uint32_t);
 
-  assert(Current == Begin + RootParametersOffset);
+  assert(Current == PartData.begin() + RootParametersOffset);
 
-  ParametersHeaders.Data = Data.substr(
+  ParametersHeaders.Data = PartData.substr(
       RootParametersOffset, NumParameters * sizeof(dxbc::RootParameterHeader));
 
   return Error::success();
diff --git a/llvm/lib/ObjectYAML/DXContainerYAML.cpp b/llvm/lib/ObjectYAML/DXContainerYAML.cpp
index 7d43e6c7f66b8..d94347a946123 100644
--- a/llvm/lib/ObjectYAML/DXContainerYAML.cpp
+++ b/llvm/lib/ObjectYAML/DXContainerYAML.cpp
@@ -40,6 +40,8 @@ DXContainerYAML::RootSignatureYamlDesc::create(
   RootSigDesc.Version = Data.getVersion();
   RootSigDesc.NumStaticSamplers = Data.getNumStaticSamplers();
   RootSigDesc.StaticSamplersOffset = Data.getStaticSamplersOffset();
+  RootSigDesc.NumRootParameters = Data.getNumRootParameters();
+  RootSigDesc.RootParametersOffset = Data.getRootParametersOffset();
 
   uint32_t Flags = Data.getFlags();
   for (const auto &PH : Data.param_headers()) {
@@ -51,13 +53,13 @@ DXContainerYAML::RootSignatureYamlDesc::create(
       return createStringError(std::errc::invalid_argument,
                                "Invalid value for parameter type");
 
-    NewP.Type = (dxbc::RootParameterType)PH.ParameterType;
+    NewP.Type = PH.ParameterType;
 
     if (!dxbc::isValidShaderVisibility(PH.ShaderVisibility))
       return createStringError(std::errc::invalid_argument,
                                "Invalid value for shader visibility");
 
-    NewP.Visibility = (dxbc::ShaderVisibility)PH.ShaderVisibility;
+    NewP.Visibility = PH.ShaderVisibility;
 
     llvm::Expected<object::DirectX::RootParameterView> ParamViewOrErr =
         Data.getParameter(PH);
@@ -257,6 +259,8 @@ void MappingTraits<DXContainerYAML::Signature>::mapping(
 void MappingTraits<DXContainerYAML::RootSignatureYamlDesc>::mapping(
     IO &IO, DXContainerYAML::RootSignatureYamlDesc &S) {
   IO.mapRequired("Version", S.Version);
+  IO.mapRequired("NumRootParameters", S.NumRootParameters);
+  IO.mapRequired("RootParametersOffset", S.RootParametersOffset);
   IO.mapRequired("NumStaticSamplers", S.NumStaticSamplers);
   IO.mapRequired("StaticSamplersOffset", S.StaticSamplersOffset);
   IO.mapRequired("Parameters", S.Parameters);
@@ -385,18 +389,6 @@ void ScalarEnumerationTraits<dxbc::SigComponentType>::enumeration(
     IO.enumCase(Value, E.Name.str().c_str(), E.Value);
 }
 
-void ScalarEnumerationTraits<dxbc::RootParameterType>::enumeration(
-    IO &IO, dxbc::RootParameterType &Value) {
-  for (const auto &E : dxbc::getRootParameterTypes())
-    IO.enumCase(Value, E.Name.str().c_str(), E.Value);
-}
-
-void ScalarEnumerationTraits<dxbc::ShaderVisibility>::enumeration(
-    IO &IO, dxbc::ShaderVisibility &Value) {
-  for (const auto &E : dxbc::getShaderVisibility())
-    IO.enumCase(Value, E.Name.str().c_str(), E.Value);
-}
-
 } // namespace yaml
 
 void DXContainerYAML::PSVInfo::mapInfoForVersion(yaml::IO &IO) {
diff --git a/llvm/test/CodeGen/DirectX/ContainerData/RootSignature-Flags.ll b/llvm/test/CodeGen/DirectX/ContainerData/RootSignature-Flags.ll
index 1ca6ebb7ddab8..e81679732a5d8 100644
--- a/llvm/test/CodeGen/DirectX/ContainerData/RootSignature-Flags.ll
+++ b/llvm/test/CodeGen/DirectX/ContainerData/RootSignature-Flags.ll
@@ -22,6 +22,8 @@ attributes #0 = { "hlsl.numthreads"="1,1,1" "hlsl.shader"="compute" }
 ; DXC-NEXT:    Size:            24
 ; DXC-NEXT:    RootSignature:
 ; DXC-NEXT:      Version:         2
+; DXC-NEXT:      NumRootParameters: 0
+; DXC-NEXT:      RootParametersOffset: 24
 ; DXC-NEXT:      NumStaticSamplers: 0
 ; DXC-NEXT:      StaticSamplersOffset: 0
 ; DXC-NEXT:      Parameters: []
diff --git a/llvm/test/ObjectYAML/DXContainer/RootSignature-Flags.yaml b/llvm/test/ObjectYAML/DXContainer/RootSignature-Flags.yaml
index 0d7902afdaa66..831664de2c9d9 100644
--- a/llvm/test/ObjectYAML/DXContainer/RootSignature-Flags.yaml
+++ b/llvm/test/ObjectYAML/DXContainer/RootSignature-Flags.yaml
@@ -14,6 +14,8 @@ Parts:
     Size:            24
     RootSignature:
       Version: 2
+      NumRootParameters: 0
+      RootParametersOffset: 24
       NumStaticSamplers: 4
       StaticSamplersOffset: 5
       Parameters: []
@@ -24,6 +26,8 @@ Parts:
 # CHECK-NEXT:    Size:            24
 # CHECK-NEXT:    RootSignature:
 # CHECK-NEXT:      Version: 2
+# CHECK-NEXT:      NumRootParameters: 0
+# CHECK-NEXT:      RootParametersOffset: 24
 # CHECK-NEXT:      NumStaticSamplers: 0
 # CHECK-NEXT:      StaticSamplersOffset: 0
 # CHECK-NEXT:      Parameters: []
diff --git a/llvm/test/ObjectYAML/DXContainer/RootSignature-InvalidType.yaml b/llvm/test/ObjectYAML/DXContainer/RootSignature-InvalidType.yaml
new file mode 100644
index 0000000000000..091e70789d956
--- /dev/null
+++ b/llvm/test/ObjectYAML/DXContainer/RootSignature-InvalidType.yaml
@@ -0,0 +1,29 @@
+# RUN: yaml2obj %s -o %t
+# RUN: not obj2yaml 2>&1 %t | FileCheck %s -DFILE=%t
+
+# CHECK: Error reading file: [[FILE]]: Invalid value for parameter type
+
+
+--- !dxcontainer
+Header:
+  Hash:            [ 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 
+                     0x0, 0x0, 0x0, 0x0, 0x0, 0x0 ]
+  Version:
+    Major:           1
+    Minor:           0
+  PartCount:       1
+  PartOffsets:     [ 60 ]
+Parts:
+  - Name:            RTS0
+    Size:            80
+    RootSignature:
+      Version: 2
+      NumRootParameters: 2
+      RootParametersOffset: 24
+      NumStaticSamplers: 0
+      StaticSamplersOffset: 64
+      Parameters:         
+      - ParameterType: 255 # INVALID
+        ShaderVisibility: 2 # Hull
+      AllowInputAssemblerInputLayout: true
+      DenyGeometryShaderRootAccess: true
diff --git a/llvm/test/ObjectYAML/DXContainer/RootSignature-InvalidVisibility.yaml b/llvm/test/ObjectYAML/DXContainer/RootSignature-InvalidVisibility.yaml
new file mode 100644
index 0000000000000..1acaf6e4e08a4
--- /dev/null
+++ b/llvm/test/ObjectYAML/DXContainer/RootSignature-InvalidVisibility.yaml
@@ -0,0 +1,33 @@
+# RUN: yaml2obj %s -o %t
+# RUN: not obj2yaml 2>&1 %t | FileCheck %s -DFILE=%t
+
+# CHECK: Error reading file: [[FILE]]: Invalid value for shader visibility
+
+
+--- !dxcontainer
+Header:
+  Hash:            [ 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 
+                     0x0, 0x0, 0x0, 0x0, 0x0, 0x0 ]
+  Version:
+    Major:           1
+    Minor:           0
+  PartCount:       1
+  PartOffsets:     [ 60 ]
+Parts:
+  - Name:            RTS0
+    Size:            80
+    RootSignature:
+      Version: 2
+      NumRootParameters: 2
+      RootParametersOffset: 24
+      NumStaticSamplers: 0
+      StaticSamplersOffset: 64
+      Parameters:         
+      - ParameterType: 1 # Constants32Bit
+        ShaderVisibility: 255 # INVALID
+        Constants:
+          Num32BitValues: 21
+          ShaderRegister: 22
+          RegisterSpace: 23   
+      AllowInputAssemblerInputLayout: true
+      DenyGeometryShaderRootAccess: true
diff --git a/llvm/test/ObjectYAML/DXContainer/RootSignature-MultipleParameters.yaml b/llvm/test/ObjectYAML/DXContainer/RootSignature-MultipleParameters.yaml
index 8d5ab5c1b0b23..d6316765e42fb 100644
--- a/llvm/test/ObjectYAML/DXContainer/RootSignature-MultipleParameters.yaml
+++ b/llvm/test/ObjectYAML/DXContainer/RootSignature-MultipleParameters.yaml
@@ -14,17 +14,19 @@ Parts:
     Size:            80
     RootSignature:
       Version: 2
+      NumRootParameters: 2
+      RootParametersOffset: 24
       NumStaticSamplers: 0
       StaticSamplersOffset: 64
       Parameters:         
-      - ParameterType: Constants32Bit
-        ShaderVisibility: Hull
+      - ParameterType: 1 # Constants32Bit
+        ShaderVisibility: 2 # Hull
         Constants:
           Num32BitValues: 16
           ShaderRegister: 15
           RegisterSpace: 14
-      - ParameterType: Constants32Bit
-        ShaderVisibility: Geometry
+      - ParameterType: 1 # Constants32Bit
+        ShaderVisibility: 4 # Geometry
         Constants:
           Num32BitValues: 21
           ShaderRegister: 22
@@ -36,17 +38,19 @@ Parts:
 # CHECK-NEXT:    Size:            80
 # CHECK-NEXT:    RootSignature:
 # CHECK-NEXT:      Version:         2
+# CHECK-NEXT:      NumRootParameters: 2
+# CHECK-NEXT:      RootParametersOffset: 24
 # CHECK-NEXT:      NumStaticSamplers: 0
 # CHECK-NEXT:      StaticSamplersOffset: 0
 # CHECK-NEXT:      Parameters:
-# CHECK-NEXT:        - ParameterType:   Constants32Bit
-# CHECK-NEXT:          ShaderVisibility: Hull
+# CHECK-NEXT:        - ParameterType:   1
+# CHECK-NEXT:          ShaderVisibility: 2
 # CHECK-NEXT:          Constants:
 # CHECK-NEXT:            Num32BitValues:  16
 # CHECK-NEXT:            RegisterSpace:   14
 # CHECK-NEXT:            ShaderRegister:  15
-# CHECK-NEXT:        - ParameterType:   Constants32Bit
-# CHECK-NEXT:          ShaderVisibility: Geometry
+# CHECK-NEXT:        - ParameterType:   1
+# CHECK-NEXT:          ShaderVisibility: 4
 # CHECK-NEXT:          Constants:
 # CHECK-NEXT:            Num32BitValues:  21
 # CHECK-NEXT:            RegisterSpace:   23
diff --git a/llvm/unittests/Object/DXContainerTest.cpp b/llvm/unittests/Object/DXContainerTest.cpp
index e8c299f4d5c08..bf32c07951520 100644
--- a/llvm/unittests/Object/DXContainerTest.cpp
+++ b/llvm/unittests/Object/DXContainerTest.cpp
@@ -7,7 +7,6 @@
 //===----------------------------------------------------------------------===//
 
 #include "llvm/Object/DXContainer.h"
-#include "../../tools/obj2yaml/dxcontainer2yaml.cpp"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/BinaryFormat/Magic.h"
 #include "llvm/ObjectYAML/DXContainerYAML.h"
@@ -908,44 +907,4 @@ TEST(RootSignature, ParseRootConstant) {
     ASSERT_EQ(Constants->RegisterSpace, 14u);
     ASSERT_EQ(Constants->Num32BitValues, 16u);
   }
-  {
-    uint8_t Buffer[] = {
-        0x44, 0x58, 0x42, 0x43, 0x32, 0x9a, 0x53, 0xd8, 0xec, 0xbe, 0x35, 0x6f,
-        0x05, 0x39, 0xe1, 0xfe, 0x31, 0x20, 0xf0, 0xc1, 0x01, 0x00, 0x00, 0x00,
-        0x85, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x24, 0x00, 0x00, 0x00,
-        0x52, 0x54, 0x53, 0x30, 0x59, 0x00, 0x00, 0x00, 0x02, 0x00, 0x00, 0x00,
-        0x01, 0x00, 0x00, 0x00, 0x18, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
-        0x2c, 0x00, 0x00, 0x00, 0x11, 0x00, 0x00, 0x00, 0xFF, 0x00, 0x00, 0x00,
-        0x02, 0x00, 0x00, 0x00, 0x24, 0x00, 0x00, 0x00, 0x0f, 0x00, 0x00, 0x00,
-        0x0e, 0x00, 0x00, 0x00, 0x10, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
-        0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
-        0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
-        0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
-        0x00};
-
-    SmallString<256> Storage;
-    raw_svector_ostream OS(Storage);
-    EXPECT_THAT_ERROR(dxcontainer2yaml(OS, getMemoryBuffer<133>(Buffer)),
-                      FailedWithMessage("Invalid value for parameter type"));
-  }
-  {
-    uint8_t Buffer[] = {
-        0x44, 0x58, 0x42, 0x43, 0x32, 0x9a, 0x53, 0xd8, 0xec, 0xbe, 0x35, 0x6f,
-        0x05, 0x39, 0xe1, 0xfe, 0x31, 0x20, 0xf0, 0xc1, 0x01, 0x00, 0x00, 0x00,
-        0x85, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x24, 0x00, 0x00, 0x00,
-        0x52, 0x54, 0x53, 0x30, 0x59, 0x00, 0x00, 0x00, 0x02, 0x00, 0x00, 0x00,
-        0x01, 0x00, 0x00, 0x00, 0x18, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
-        0x2c, 0x00, 0x00, 0x00, 0x11, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00,
-        0xFF, 0x00, 0x00, 0x00, 0x24, 0x00, 0x00, 0x00, 0x0f, 0x00, 0x00, 0x00,
-        0x0e, 0x00, 0x00, 0x00, 0x10, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
-        0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
-        0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
-        0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
-        0x00};
-
-    SmallString<256> Storage;
-    raw_svector_ostream OS(Storage);
-    EXPECT_THAT_ERROR(dxcontainer2yaml(OS, getMemoryBuffer<133>(Buffer)),
-                      FailedWithMessage("Invalid value for shader visibility"));
-  }
 }
diff --git a/llvm/unittests/ObjectYAML/DXContainerYAMLTest.cpp b/llvm/unittests/ObjectYAML/DXContainerYAMLTest.cpp
index 3e5b79be2ccff..99ca161883498 100644
--- a/llvm/unittests/ObjectYAML/DXContainerYAMLTest.cpp
+++ b/llvm/unittests/ObjectYAML/DXContainerYAMLTest.cpp
@@ -127,6 +127,8 @@ TEST(RootSignature, ParseRootFlags) {
       Size:            24
       RootSignature:
         Version:         2
+        NumRootParameters: 0
+        RootParametersOffset: 24
         NumStaticSamplers: 0
         StaticSamplersOffset: 0
         Parameters: []
@@ -165,11 +167,13 @@ TEST(RootSignature, ParseRootConstants) {
       Size:            89
       RootSignature:
         Version: 2
+        NumRootParameters: 1
+        RootParametersOffset: 24
         NumStaticSamplers: 0
         StaticSamplersOffset: 56
         Parameters:
-          - ParameterType: Constants32Bit
-            ShaderVisibility: Hull
+          - ParameterType: 1
+            ShaderVisibility: 2
             Constants:
               Num32BitValues: 16
               ShaderRegister: 15



More information about the llvm-branch-commits mailing list