[HLSL] Adding support for root descriptors in root signature metadata representation #139781

joaosaffran · 2025-05-13T19:33:18Z

adds parsing from metadata into dxcontainer binary
adds validations as described in the spec
adds testing scenarios
closes: #126638

llvmbot · 2025-05-13T19:33:50Z

@llvm/pr-subscribers-backend-directx

Author: None (joaosaffran)

Changes

adds parsing from metadata into dxcontainer binary
adds validations as described in the spec
adds testing scenarios
closes: #126638

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

7 Files Affected:

(modified) llvm/lib/Target/DirectX/DXILRootSignature.cpp (+121-1)
(modified) llvm/lib/Target/DirectX/DXILRootSignature.h (+2-1)
(added) llvm/test/CodeGen/DirectX/ContainerData/RootSignature-RootDescriptor-Invalid-Flags.ll (+18)
(added) llvm/test/CodeGen/DirectX/ContainerData/RootSignature-RootDescriptor-Invalid-RegisterKind.ll (+18)
(added) llvm/test/CodeGen/DirectX/ContainerData/RootSignature-RootDescriptor-Invalid-RegisterSpace.ll (+18)
(added) llvm/test/CodeGen/DirectX/ContainerData/RootSignature-RootDescriptor-Invalid-RegisterValue.ll (+18)
(added) llvm/test/CodeGen/DirectX/ContainerData/RootSignature-RootDescriptor.ll (+34)

diff --git a/llvm/lib/Target/DirectX/DXILRootSignature.cpp b/llvm/lib/Target/DirectX/DXILRootSignature.cpp
index 1bd816b026fec..3ee52d32eaf2d 100644
--- a/llvm/lib/Target/DirectX/DXILRootSignature.cpp
+++ b/llvm/lib/Target/DirectX/DXILRootSignature.cpp
@@ -55,6 +55,14 @@ static std::optional<uint32_t> extractMdIntValue(MDNode *Node,
   return std::nullopt;
 }
 
+static std::optional<StringRef> extractMdStringValue(MDNode *Node,
+                                                     unsigned int OpId) {
+  MDString *NodeText = cast<MDString>(Node->getOperand(OpId));
+  if (NodeText == nullptr)
+    return std::nullopt;
+  return NodeText->getString();
+}
+
 static bool parseRootFlags(LLVMContext *Ctx, mcdxbc::RootSignatureDesc &RSD,
                            MDNode *RootFlagNode) {
 
@@ -105,6 +113,56 @@ static bool parseRootConstants(LLVMContext *Ctx, mcdxbc::RootSignatureDesc &RSD,
   return false;
 }
 
+static bool parseRootDescriptors(LLVMContext *Ctx,
+                                 mcdxbc::RootSignatureDesc &RSD,
+                                 MDNode *RootDescriptorNode) {
+
+  if (RootDescriptorNode->getNumOperands() != 5)
+    return reportError(Ctx, "Invalid format for RootConstants Element");
+
+  std::optional<StringRef> ElementText =
+      extractMdStringValue(RootDescriptorNode, 0);
+  assert(!ElementText->empty());
+
+  dxbc::RootParameterHeader Header;
+  Header.ParameterType =
+      StringSwitch<uint32_t>(*ElementText)
+          .Case("RootCBV", llvm::to_underlying(dxbc::RootParameterType::CBV))
+          .Case("RootSRV", llvm::to_underlying(dxbc::RootParameterType::SRV))
+          .Case("RootUAV", llvm::to_underlying(dxbc::RootParameterType::UAV));
+
+  if (std::optional<uint32_t> Val = extractMdIntValue(RootDescriptorNode, 1))
+    Header.ShaderVisibility = *Val;
+  else
+    return reportError(Ctx, "Invalid value for ShaderVisibility");
+
+  dxbc::RTS0::v1::RootDescriptor Descriptor;
+  if (std::optional<uint32_t> Val = extractMdIntValue(RootDescriptorNode, 2))
+    Descriptor.ShaderRegister = *Val;
+  else
+    return reportError(Ctx, "Invalid value for ShaderRegister");
+
+  if (std::optional<uint32_t> Val = extractMdIntValue(RootDescriptorNode, 3))
+    Descriptor.RegisterSpace = *Val;
+  else
+    return reportError(Ctx, "Invalid value for RegisterSpace");
+
+  if (RSD.Version == 1) {
+    RSD.ParametersContainer.addParameter(Header, Descriptor);
+    return false;
+  }
+  assert(RSD.Version > 1);
+  dxbc::RTS0::v2::RootDescriptor DescriptorV2(Descriptor);
+
+  if (std::optional<uint32_t> Val = extractMdIntValue(RootDescriptorNode, 4))
+    DescriptorV2.Flags = *Val;
+  else
+    return reportError(Ctx, "Invalid value for Root Descriptor Flags");
+
+  RSD.ParametersContainer.addParameter(Header, DescriptorV2);
+  return false;
+}
+
 static bool parseRootSignatureElement(LLVMContext *Ctx,
                                       mcdxbc::RootSignatureDesc &RSD,
                                       MDNode *Element) {
@@ -116,6 +174,9 @@ static bool parseRootSignatureElement(LLVMContext *Ctx,
       StringSwitch<RootSignatureElementKind>(ElementText->getString())
           .Case("RootFlags", RootSignatureElementKind::RootFlags)
           .Case("RootConstants", RootSignatureElementKind::RootConstants)
+          .Case("RootCBV", RootSignatureElementKind::RootDescriptors)
+          .Case("RootSRV", RootSignatureElementKind::RootDescriptors)
+          .Case("RootUAV", RootSignatureElementKind::RootDescriptors)
           .Default(RootSignatureElementKind::Error);
 
   switch (ElementKind) {
@@ -124,7 +185,8 @@ static bool parseRootSignatureElement(LLVMContext *Ctx,
     return parseRootFlags(Ctx, RSD, Element);
   case RootSignatureElementKind::RootConstants:
     return parseRootConstants(Ctx, RSD, Element);
-    break;
+  case RootSignatureElementKind::RootDescriptors:
+    return parseRootDescriptors(Ctx, RSD, Element);
   case RootSignatureElementKind::Error:
     return reportError(Ctx, "Invalid Root Signature Element: " +
                                 ElementText->getString());
@@ -155,6 +217,16 @@ static bool verifyVersion(uint32_t Version) {
   return (Version == 1 || Version == 2);
 }
 
+static bool verifyRegisterValue(uint32_t RegisterValue) {
+  return !(RegisterValue == 0xFFFFFFFF);
+}
+
+static bool verifyRegisterSpace(uint32_t RegisterSpace) {
+  return !(RegisterSpace >= 0xFFFFFFF0 && RegisterSpace <= 0xFFFFFFFF);
+}
+
+static bool verifyDescriptorFlag(uint32_t Flags) { return (Flags & ~0xE) == 0; }
+
 static bool validate(LLVMContext *Ctx, const mcdxbc::RootSignatureDesc &RSD) {
 
   if (!verifyVersion(RSD.Version)) {
@@ -172,6 +244,39 @@ static bool validate(LLVMContext *Ctx, const mcdxbc::RootSignatureDesc &RSD) {
 
     assert(dxbc::isValidParameterType(Info.Header.ParameterType) &&
            "Invalid value for ParameterType");
+
+    auto P = RSD.ParametersContainer.getParameter(&Info);
+    if (!P)
+      return reportError(Ctx, "Cannot locate parameter from Header Info");
+
+    if (std::holds_alternative<const dxbc::RTS0::v1::RootDescriptor *>(*P)) {
+      auto *Descriptor =
+          std::get<const dxbc::RTS0::v1::RootDescriptor *>(P.value());
+
+      if (!verifyRegisterValue(Descriptor->ShaderRegister))
+        return reportValueError(Ctx, "ShaderRegister",
+                                Descriptor->ShaderRegister);
+
+      if (!verifyRegisterSpace(Descriptor->RegisterSpace))
+        return reportValueError(Ctx, "RegisterSpace",
+                                Descriptor->RegisterSpace);
+
+    } else if (std::holds_alternative<const dxbc::RTS0::v2::RootDescriptor *>(
+                   *P)) {
+      auto *Descriptor =
+          std::get<const dxbc::RTS0::v2::RootDescriptor *>(P.value());
+
+      if (!verifyRegisterValue(Descriptor->ShaderRegister))
+        return reportValueError(Ctx, "ShaderRegister",
+                                Descriptor->ShaderRegister);
+
+      if (!verifyRegisterSpace(Descriptor->RegisterSpace))
+        return reportValueError(Ctx, "RegisterSpace",
+                                Descriptor->RegisterSpace);
+
+      if (!verifyDescriptorFlag(Descriptor->Flags))
+        return reportValueError(Ctx, "DescriptorFlag", Descriptor->Flags);
+    }
   }
 
   return false;
@@ -308,6 +413,21 @@ PreservedAnalyses RootSignatureAnalysisPrinter::run(Module &M,
            << "Shader Register: " << Constants->ShaderRegister << "\n";
         OS << indent(Space + 2)
            << "Num 32 Bit Values: " << Constants->Num32BitValues << "\n";
+      } else if (std::holds_alternative<const dxbc::RTS0::v1::RootDescriptor *>(
+                     *P)) {
+        auto *Constants = std::get<const dxbc::RTS0::v1::RootDescriptor *>(*P);
+        OS << indent(Space + 2)
+           << "Register Space: " << Constants->RegisterSpace << "\n";
+        OS << indent(Space + 2)
+           << "Shader Register: " << Constants->ShaderRegister << "\n";
+      } else if (std::holds_alternative<const dxbc::RTS0::v2::RootDescriptor *>(
+                     *P)) {
+        auto *Constants = std::get<const dxbc::RTS0::v2::RootDescriptor *>(*P);
+        OS << indent(Space + 2)
+           << "Register Space: " << Constants->RegisterSpace << "\n";
+        OS << indent(Space + 2)
+           << "Shader Register: " << Constants->ShaderRegister << "\n";
+        OS << indent(Space + 2) << "Flags: " << Constants->Flags << "\n";
       }
     }
     Space--;
diff --git a/llvm/lib/Target/DirectX/DXILRootSignature.h b/llvm/lib/Target/DirectX/DXILRootSignature.h
index 93ec614f1ab85..b8742d1b1fdfd 100644
--- a/llvm/lib/Target/DirectX/DXILRootSignature.h
+++ b/llvm/lib/Target/DirectX/DXILRootSignature.h
@@ -27,7 +27,8 @@ namespace dxil {
 enum class RootSignatureElementKind {
   Error = 0,
   RootFlags = 1,
-  RootConstants = 2
+  RootConstants = 2,
+  RootDescriptors = 3
 };
 class RootSignatureAnalysis : public AnalysisInfoMixin<RootSignatureAnalysis> {
   friend AnalysisInfoMixin<RootSignatureAnalysis>;
diff --git a/llvm/test/CodeGen/DirectX/ContainerData/RootSignature-RootDescriptor-Invalid-Flags.ll b/llvm/test/CodeGen/DirectX/ContainerData/RootSignature-RootDescriptor-Invalid-Flags.ll
new file mode 100644
index 0000000000000..4229981240918
--- /dev/null
+++ b/llvm/test/CodeGen/DirectX/ContainerData/RootSignature-RootDescriptor-Invalid-Flags.ll
@@ -0,0 +1,18 @@
+; RUN: not opt -passes='print<dxil-root-signature>' %s -S -o - 2>&1 | FileCheck %s
+
+target triple = "dxil-unknown-shadermodel6.0-compute"
+
+
+; CHECK: error: Invalid value for DescriptorFlag: 3
+; CHECK-NOT: Root Signature Definitions
+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 = !{ !5 } ; list of root signature elements
+!5 = !{ !"RootCBV", i32 0, i32 1, i32 2, i32 3  }
diff --git a/llvm/test/CodeGen/DirectX/ContainerData/RootSignature-RootDescriptor-Invalid-RegisterKind.ll b/llvm/test/CodeGen/DirectX/ContainerData/RootSignature-RootDescriptor-Invalid-RegisterKind.ll
new file mode 100644
index 0000000000000..4aed84efbe2bc
--- /dev/null
+++ b/llvm/test/CodeGen/DirectX/ContainerData/RootSignature-RootDescriptor-Invalid-RegisterKind.ll
@@ -0,0 +1,18 @@
+; RUN: not opt -passes='print<dxil-root-signature>' %s -S -o - 2>&1 | FileCheck %s
+
+target triple = "dxil-unknown-shadermodel6.0-compute"
+
+
+; CHECK: error:  Invalid Root Signature Element: Invalid 
+; CHECK-NOT: Root Signature Definitions
+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 = !{ !5 } ; list of root signature elements
+!5 = !{ !"Invalid", i32 0, i32 1, i32 2, i32 3  }
diff --git a/llvm/test/CodeGen/DirectX/ContainerData/RootSignature-RootDescriptor-Invalid-RegisterSpace.ll b/llvm/test/CodeGen/DirectX/ContainerData/RootSignature-RootDescriptor-Invalid-RegisterSpace.ll
new file mode 100644
index 0000000000000..020d117ba45dc
--- /dev/null
+++ b/llvm/test/CodeGen/DirectX/ContainerData/RootSignature-RootDescriptor-Invalid-RegisterSpace.ll
@@ -0,0 +1,18 @@
+; RUN: not opt -passes='print<dxil-root-signature>' %s -S -o - 2>&1 | FileCheck %s
+
+target triple = "dxil-unknown-shadermodel6.0-compute"
+
+
+; CHECK: error:  Invalid value for RegisterSpace: 4294967280
+; CHECK-NOT: Root Signature Definitions
+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 = !{ !5 } ; list of root signature elements
+!5 = !{ !"RootCBV", i32 0, i32 1, i32 4294967280, i32 0  }
diff --git a/llvm/test/CodeGen/DirectX/ContainerData/RootSignature-RootDescriptor-Invalid-RegisterValue.ll b/llvm/test/CodeGen/DirectX/ContainerData/RootSignature-RootDescriptor-Invalid-RegisterValue.ll
new file mode 100644
index 0000000000000..edb8b943c6e35
--- /dev/null
+++ b/llvm/test/CodeGen/DirectX/ContainerData/RootSignature-RootDescriptor-Invalid-RegisterValue.ll
@@ -0,0 +1,18 @@
+; RUN: not opt -passes='print<dxil-root-signature>' %s -S -o - 2>&1 | FileCheck %s
+
+target triple = "dxil-unknown-shadermodel6.0-compute"
+
+
+; CHECK: error: Invalid value for ShaderRegister: 4294967295 
+; CHECK-NOT: Root Signature Definitions
+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 = !{ !5 } ; list of root signature elements
+!5 = !{ !"RootCBV", i32 0, i32 4294967295, i32 2, i32 3  }
diff --git a/llvm/test/CodeGen/DirectX/ContainerData/RootSignature-RootDescriptor.ll b/llvm/test/CodeGen/DirectX/ContainerData/RootSignature-RootDescriptor.ll
new file mode 100644
index 0000000000000..9217945855cd9
--- /dev/null
+++ b/llvm/test/CodeGen/DirectX/ContainerData/RootSignature-RootDescriptor.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 = !{ !5 } ; list of root signature elements
+!5 = !{ !"RootCBV", i32 0, i32 1, i32 2, i32 8  }
+
+; DXC:  - Name:            RTS0
+; DXC-NEXT:    Size:            48
+; DXC-NEXT:    RootSignature:
+; DXC-NEXT:      Version:         2
+; DXC-NEXT:      NumRootParameters: 1 
+; DXC-NEXT:      RootParametersOffset: 24 
+; DXC-NEXT:      NumStaticSamplers: 0
+; DXC-NEXT:      StaticSamplersOffset: 0
+; DXC-NEXT:      Parameters:
+; DXC-NEXT:        - ParameterType:   2
+; DXC-NEXT:          ShaderVisibility: 0
+; DXC-NEXT:          Descriptor:
+; DXC-NEXT:            RegisterSpace: 2
+; DXC-NEXT:            ShaderRegister: 1
+; DXC-NEXT:            DATA_STATIC: true

inbelic · 2025-05-20T17:55:08Z

llvm/lib/Target/DirectX/DXILRootSignature.cpp

+      StringSwitch<uint32_t>(*ElementText)
+          .Case("RootCBV", llvm::to_underlying(dxbc::RootParameterType::CBV))
+          .Case("RootSRV", llvm::to_underlying(dxbc::RootParameterType::SRV))
+          .Case("RootUAV", llvm::to_underlying(dxbc::RootParameterType::UAV));


Without a default case, this is equivalent to llvm_unreachable right?

inbelic · 2025-05-20T17:58:33Z

llvm/lib/Target/DirectX/DXILRootSignature.cpp

+    return reportError(Ctx, "Invalid format for RootConstants Element");
+
+  std::optional<StringRef> ElementText =
+      extractMdStringValue(RootDescriptorNode, 0);


We should add a check that this is not std::nullopt here and create an error. Otherwise, the below assert or the deference below will caush a crash.

inbelic · 2025-05-20T17:59:29Z

llvm/lib/Target/DirectX/DXILRootSignature.cpp

+  return !(RegisterSpace >= 0xFFFFFFF0 && RegisterSpace <= 0xFFFFFFFF);
+}
+
+static bool verifyDescriptorFlag(uint32_t Flags) { return (Flags & ~0xE) == 0; }


self-note: this is only called on v2, so it implicitly handles the different verification based on version.

inbelic · 2025-05-20T18:01:49Z

llvm/lib/Target/DirectX/DXILRootSignature.cpp

+      } else if (std::holds_alternative<const dxbc::RTS0::v1::RootDescriptor *>(
+                     *P)) {
+        auto *Constants = std::get<const dxbc::RTS0::v1::RootDescriptor *>(*P);
+        OS << indent(Space + 2)
+           << "Register Space: " << Constants->RegisterSpace << "\n";
+        OS << indent(Space + 2)
+           << "Shader Register: " << Constants->ShaderRegister << "\n";
+      } else if (std::holds_alternative<const dxbc::RTS0::v2::RootDescriptor *>(
+                     *P)) {
+        auto *Constants = std::get<const dxbc::RTS0::v2::RootDescriptor *>(*P);
+        OS << indent(Space + 2)
+           << "Register Space: " << Constants->RegisterSpace << "\n";
+        OS << indent(Space + 2)
+           << "Shader Register: " << Constants->ShaderRegister << "\n";
+        OS << indent(Space + 2) << "Flags: " << Constants->Flags << "\n";


This doesn't seem to be invoked in any of the test cases?

The test cases seems to only output the YAML representation

joaosaffran added 4 commits May 13, 2025 02:07

adding support for root descriptors

f875555

removing none as a flag option

4f7f998

adding tests

3eb5e10

clean up and add more tests

58e1789

joaosaffran assigned bogner and inbelic May 13, 2025

llvmbot added the backend:DirectX label May 13, 2025

joaosaffran linked an issue May 14, 2025 that may be closed by this pull request

[HLSL] Add Root Descriptor element support to dxcontainer generation #126638

Open

4 tasks

inbelic reviewed May 20, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[HLSL] Adding support for root descriptors in root signature metadata representation #139781

[HLSL] Adding support for root descriptors in root signature metadata representation #139781

Uh oh!

joaosaffran commented May 13, 2025

Uh oh!

llvmbot commented May 13, 2025

Uh oh!

inbelic May 20, 2025

Uh oh!

inbelic May 20, 2025

Uh oh!

inbelic May 20, 2025

Uh oh!

inbelic May 20, 2025

Uh oh!

Uh oh!

[HLSL] Adding support for root descriptors in root signature metadata representation #139781

Are you sure you want to change the base?

[HLSL] Adding support for root descriptors in root signature metadata representation #139781

Uh oh!

Conversation

joaosaffran commented May 13, 2025

Uh oh!

llvmbot commented May 13, 2025

Uh oh!

inbelic May 20, 2025

Choose a reason for hiding this comment

Uh oh!

inbelic May 20, 2025

Choose a reason for hiding this comment

Uh oh!

inbelic May 20, 2025

Choose a reason for hiding this comment

Uh oh!

inbelic May 20, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!