[llvm-branch-commits] [clang] [llvm] [HLSL][RootSignature] Add Metadata generation of Root Signatures for Attr (PR #125131)

Finn Plummer via llvm-branch-commits llvm-branch-commits at lists.llvm.org
Thu Jan 30 15:15:03 PST 2025


https://github.com/inbelic updated https://github.com/llvm/llvm-project/pull/125131

>From 96a959b5c1dd4d46b72344902d697a40100d8046 Mon Sep 17 00:00:00 2001
From: Finn Plummer <canadienfinn at gmail.com>
Date: Wed, 29 Jan 2025 19:40:08 +0000
Subject: [PATCH 1/7] add basic empty root signature

---
 clang/lib/CodeGen/CGHLSLRuntime.cpp       | 21 +++++++++++++++++++++
 clang/test/CodeGenHLSL/RootSignature.hlsl | 19 +++++++++++++++++++
 2 files changed, 40 insertions(+)
 create mode 100644 clang/test/CodeGenHLSL/RootSignature.hlsl

diff --git a/clang/lib/CodeGen/CGHLSLRuntime.cpp b/clang/lib/CodeGen/CGHLSLRuntime.cpp
index c354e58e15f4bb..ff608323e9ac3a 100644
--- a/clang/lib/CodeGen/CGHLSLRuntime.cpp
+++ b/clang/lib/CodeGen/CGHLSLRuntime.cpp
@@ -119,6 +119,20 @@ GlobalVariable *replaceBuffer(CGHLSLRuntime::Buffer &Buf) {
   return CBGV;
 }
 
+void addRootSignature(llvm::Function *Fn, llvm::Module &M) {
+  auto &Ctx = M.getContext();
+  IRBuilder<> B(M.getContext());
+
+  MDNode *ExampleRootSignature = MDNode::get(Ctx, {});
+
+  MDNode *ExamplePairing = MDNode::get(Ctx, {ValueAsMetadata::get(Fn),
+                                             ExampleRootSignature});
+
+  StringRef RootSignatureValKey = "dx.rootsignatures";
+  auto *RootSignatureValMD = M.getOrInsertNamedMetadata(RootSignatureValKey);
+  RootSignatureValMD->addOperand(ExamplePairing);
+}
+
 } // namespace
 
 llvm::Type *CGHLSLRuntime::convertHLSLSpecificType(const Type *T) {
@@ -453,6 +467,13 @@ void CGHLSLRuntime::emitEntryFunction(const FunctionDecl *FD,
   // FIXME: Handle codegen for return type semantics.
   // See: https://github.com/llvm/llvm-project/issues/57875
   B.CreateRetVoid();
+
+  // Add and identify root signature to function, if applicable
+  const AttrVec &Attrs = FD->getAttrs();
+  for (const Attr *Attr : Attrs) {
+    if (isa<HLSLRootSignatureAttr>(Attr))
+      addRootSignature(EntryFn, M);
+  }
 }
 
 void CGHLSLRuntime::setHLSLFunctionAttributes(const FunctionDecl *FD,
diff --git a/clang/test/CodeGenHLSL/RootSignature.hlsl b/clang/test/CodeGenHLSL/RootSignature.hlsl
new file mode 100644
index 00000000000000..1ea9ab7aaa2c35
--- /dev/null
+++ b/clang/test/CodeGenHLSL/RootSignature.hlsl
@@ -0,0 +1,19 @@
+// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.3-library -emit-llvm -o - %s | FileCheck %s
+
+// CHECK: !dx.rootsignatures = !{![[#FIRST_ENTRY:]], ![[#SECOND_ENTRY:]]}
+// CHECK-DAG: ![[#FIRST_ENTRY]] = !{ptr @FirstEntry, ![[#RS:]]}
+// CHECK-DAG: ![[#SECOND_ENTRY]] = !{ptr @SecondEntry, ![[#RS:]]}
+// CHECK-DAG: ![[#RS]] = !{}
+
+[shader("compute"), RootSignature("")]
+[numthreads(1,1,1)]
+void FirstEntry() {}
+
+[shader("compute"), RootSignature("DescriptorTable()")]
+[numthreads(1,1,1)]
+void SecondEntry() {}
+
+// Sanity test to ensure to root is added for this function
+[shader("compute")]
+[numthreads(1,1,1)]
+void ThirdEntry() {}

>From 3f292a141ea90674301a3c23d02d2677fc3b8b23 Mon Sep 17 00:00:00 2001
From: Finn Plummer <canadienfinn at gmail.com>
Date: Wed, 29 Jan 2025 19:57:48 +0000
Subject: [PATCH 2/7] pass down the actual root elements

- test that we have the correct number of elements
---
 clang/lib/CodeGen/CGHLSLRuntime.cpp       | 17 ++++++++++++-----
 clang/test/CodeGenHLSL/RootSignature.hlsl |  9 +++++----
 2 files changed, 17 insertions(+), 9 deletions(-)

diff --git a/clang/lib/CodeGen/CGHLSLRuntime.cpp b/clang/lib/CodeGen/CGHLSLRuntime.cpp
index ff608323e9ac3a..4c9adcd8a90536 100644
--- a/clang/lib/CodeGen/CGHLSLRuntime.cpp
+++ b/clang/lib/CodeGen/CGHLSLRuntime.cpp
@@ -119,11 +119,18 @@ GlobalVariable *replaceBuffer(CGHLSLRuntime::Buffer &Buf) {
   return CBGV;
 }
 
-void addRootSignature(llvm::Function *Fn, llvm::Module &M) {
+void addRootSignature(
+    ArrayRef<llvm::hlsl::root_signature::RootElement> Elements,
+    llvm::Function *Fn, llvm::Module &M) {
   auto &Ctx = M.getContext();
-  IRBuilder<> B(M.getContext());
 
-  MDNode *ExampleRootSignature = MDNode::get(Ctx, {});
+  SmallVector<Metadata *> GeneratedMetadata;
+  for (auto Element : Elements) {
+    MDNode *ExampleRootElement = MDNode::get(Ctx, {});
+    GeneratedMetadata.push_back(ExampleRootElement);
+  }
+
+  MDNode *ExampleRootSignature = MDNode::get(Ctx, GeneratedMetadata);
 
   MDNode *ExamplePairing = MDNode::get(Ctx, {ValueAsMetadata::get(Fn),
                                              ExampleRootSignature});
@@ -471,8 +478,8 @@ void CGHLSLRuntime::emitEntryFunction(const FunctionDecl *FD,
   // Add and identify root signature to function, if applicable
   const AttrVec &Attrs = FD->getAttrs();
   for (const Attr *Attr : Attrs) {
-    if (isa<HLSLRootSignatureAttr>(Attr))
-      addRootSignature(EntryFn, M);
+    if (const auto *RSAttr = dyn_cast<HLSLRootSignatureAttr>(Attr))
+      addRootSignature(RSAttr->getElements(), EntryFn, M);
   }
 }
 
diff --git a/clang/test/CodeGenHLSL/RootSignature.hlsl b/clang/test/CodeGenHLSL/RootSignature.hlsl
index 1ea9ab7aaa2c35..63c0505e224f0a 100644
--- a/clang/test/CodeGenHLSL/RootSignature.hlsl
+++ b/clang/test/CodeGenHLSL/RootSignature.hlsl
@@ -1,9 +1,10 @@
 // RUN: %clang_cc1 -triple dxil-pc-shadermodel6.3-library -emit-llvm -o - %s | FileCheck %s
 
-// CHECK: !dx.rootsignatures = !{![[#FIRST_ENTRY:]], ![[#SECOND_ENTRY:]]}
-// CHECK-DAG: ![[#FIRST_ENTRY]] = !{ptr @FirstEntry, ![[#RS:]]}
-// CHECK-DAG: ![[#SECOND_ENTRY]] = !{ptr @SecondEntry, ![[#RS:]]}
-// CHECK-DAG: ![[#RS]] = !{}
+// CHECK-DAG: ![[#EMPTY:]] = !{}
+// CHECK-DAG: ![[#SECOND_RS:]] = !{![[#EMPTY]]}
+// CHECK-DAG: ![[#SECOND_ENTRY:]] = !{ptr @SecondEntry, ![[#SECOND_RS]]}
+// CHECK-DAG: ![[#FIRST_ENTRY:]] = !{ptr @FirstEntry, ![[#EMPTY]]}
+// CHECK-DAG: !dx.rootsignatures = !{![[#FIRST_ENTRY]], ![[#SECOND_ENTRY]]}
 
 [shader("compute"), RootSignature("")]
 [numthreads(1,1,1)]

>From dff2f008d4bed8ceb32569a93b86f16d964dc0a8 Mon Sep 17 00:00:00 2001
From: Finn Plummer <canadienfinn at gmail.com>
Date: Wed, 29 Jan 2025 22:50:13 +0000
Subject: [PATCH 3/7] introduce a MetadataBuilder to handle the construction of
 nodes

---
 clang/lib/CodeGen/CGHLSLRuntime.cpp           | 16 ++--
 clang/test/CodeGenHLSL/RootSignature.hlsl     | 17 ++--
 .../llvm/Frontend/HLSL/HLSLRootSignature.h    | 24 ++++++
 llvm/lib/Frontend/HLSL/CMakeLists.txt         |  1 +
 llvm/lib/Frontend/HLSL/HLSLRootSignature.cpp  | 77 +++++++++++++++++++
 5 files changed, 118 insertions(+), 17 deletions(-)
 create mode 100644 llvm/lib/Frontend/HLSL/HLSLRootSignature.cpp

diff --git a/clang/lib/CodeGen/CGHLSLRuntime.cpp b/clang/lib/CodeGen/CGHLSLRuntime.cpp
index 4c9adcd8a90536..168bda18eb720d 100644
--- a/clang/lib/CodeGen/CGHLSLRuntime.cpp
+++ b/clang/lib/CodeGen/CGHLSLRuntime.cpp
@@ -124,20 +124,14 @@ void addRootSignature(
     llvm::Function *Fn, llvm::Module &M) {
   auto &Ctx = M.getContext();
 
-  SmallVector<Metadata *> GeneratedMetadata;
-  for (auto Element : Elements) {
-    MDNode *ExampleRootElement = MDNode::get(Ctx, {});
-    GeneratedMetadata.push_back(ExampleRootElement);
-  }
-
-  MDNode *ExampleRootSignature = MDNode::get(Ctx, GeneratedMetadata);
-
-  MDNode *ExamplePairing = MDNode::get(Ctx, {ValueAsMetadata::get(Fn),
-                                             ExampleRootSignature});
+  llvm::hlsl::root_signature::MetadataBuilder Builder(Ctx, Elements);
+  MDNode *RootSignature = Builder.BuildRootSignature();
+  MDNode *FnPairing = MDNode::get(Ctx, {ValueAsMetadata::get(Fn),
+                                        RootSignature});
 
   StringRef RootSignatureValKey = "dx.rootsignatures";
   auto *RootSignatureValMD = M.getOrInsertNamedMetadata(RootSignatureValKey);
-  RootSignatureValMD->addOperand(ExamplePairing);
+  RootSignatureValMD->addOperand(FnPairing);
 }
 
 } // namespace
diff --git a/clang/test/CodeGenHLSL/RootSignature.hlsl b/clang/test/CodeGenHLSL/RootSignature.hlsl
index 63c0505e224f0a..659631133ad9b5 100644
--- a/clang/test/CodeGenHLSL/RootSignature.hlsl
+++ b/clang/test/CodeGenHLSL/RootSignature.hlsl
@@ -1,16 +1,17 @@
 // RUN: %clang_cc1 -triple dxil-pc-shadermodel6.3-library -emit-llvm -o - %s | FileCheck %s
 
 // CHECK-DAG: ![[#EMPTY:]] = !{}
-// CHECK-DAG: ![[#SECOND_RS:]] = !{![[#EMPTY]]}
-// CHECK-DAG: ![[#SECOND_ENTRY:]] = !{ptr @SecondEntry, ![[#SECOND_RS]]}
-// CHECK-DAG: ![[#FIRST_ENTRY:]] = !{ptr @FirstEntry, ![[#EMPTY]]}
-// CHECK-DAG: !dx.rootsignatures = !{![[#FIRST_ENTRY]], ![[#SECOND_ENTRY]]}
-
 [shader("compute"), RootSignature("")]
 [numthreads(1,1,1)]
 void FirstEntry() {}
 
-[shader("compute"), RootSignature("DescriptorTable()")]
+// CHECK-DAG: ![[#TABLE:]] = !{!"DescriptorTable"}
+// CHECK-DAG: ![[#SECOND_RS:]] = !{![[#TABLE]]}
+
+#define SampleDescriptorTable \
+  "DescriptorTable( " \
+  ")"
+[shader("compute"), RootSignature(SampleDescriptorTable)]
 [numthreads(1,1,1)]
 void SecondEntry() {}
 
@@ -18,3 +19,7 @@ void SecondEntry() {}
 [shader("compute")]
 [numthreads(1,1,1)]
 void ThirdEntry() {}
+
+// CHECK-DAG: ![[#FIRST_ENTRY:]] = !{ptr @FirstEntry, ![[#EMPTY]]}
+// CHECK-DAG: ![[#SECOND_ENTRY:]] = !{ptr @SecondEntry, ![[#SECOND_RS]]}
+// CHECK-DAG: !dx.rootsignatures = !{![[#FIRST_ENTRY]], ![[#SECOND_ENTRY]]}
diff --git a/llvm/include/llvm/Frontend/HLSL/HLSLRootSignature.h b/llvm/include/llvm/Frontend/HLSL/HLSLRootSignature.h
index 511ffd96a6d16c..433cd9bc106f6d 100644
--- a/llvm/include/llvm/Frontend/HLSL/HLSLRootSignature.h
+++ b/llvm/include/llvm/Frontend/HLSL/HLSLRootSignature.h
@@ -14,10 +14,16 @@
 #ifndef LLVM_FRONTEND_HLSL_HLSLROOTSIGNATURE_H
 #define LLVM_FRONTEND_HLSL_HLSLROOTSIGNATURE_H
 
+#include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/STLForwardCompat.h"
 #include "llvm/Support/DXILABI.h"
 #include <variant>
 
 namespace llvm {
+class LLVMContext;
+class MDNode;
+class Metadata;
+
 namespace hlsl {
 namespace root_signature {
 
@@ -122,6 +128,24 @@ using RootElement = std::variant<DescriptorTable, DescriptorTableClause>;
 using ParamType = std::variant<uint32_t *, DescriptorRangeOffset *,
                                DescriptorRangeFlags *, ShaderVisibility *>;
 
+class MetadataBuilder {
+ public:
+  MetadataBuilder(llvm::LLVMContext &Ctx, ArrayRef<RootElement> Elements)
+   : Ctx(Ctx), Elements(Elements) {}
+
+  // Iterates through the elements and builds the respective nodes
+  MDNode *BuildRootSignature();
+
+ private:
+  // Define the various builders for the different metadata types
+  MDNode *BuildDescriptorTable(const DescriptorTable &Table);
+  MDNode *BuildDescriptorTableClause(const DescriptorTableClause &Clause);
+
+  llvm::LLVMContext &Ctx;
+  ArrayRef<RootElement> Elements;
+  SmallVector<Metadata *> GeneratedMetadata;
+};
+
 } // namespace root_signature
 } // namespace hlsl
 } // namespace llvm
diff --git a/llvm/lib/Frontend/HLSL/CMakeLists.txt b/llvm/lib/Frontend/HLSL/CMakeLists.txt
index eda6cb8e69a497..739bfef8dbc37d 100644
--- a/llvm/lib/Frontend/HLSL/CMakeLists.txt
+++ b/llvm/lib/Frontend/HLSL/CMakeLists.txt
@@ -1,5 +1,6 @@
 add_llvm_component_library(LLVMFrontendHLSL
   HLSLResource.cpp
+  HLSLRootSignature.cpp
 
   ADDITIONAL_HEADER_DIRS
   ${LLVM_MAIN_INCLUDE_DIR}/llvm/Frontend
diff --git a/llvm/lib/Frontend/HLSL/HLSLRootSignature.cpp b/llvm/lib/Frontend/HLSL/HLSLRootSignature.cpp
new file mode 100644
index 00000000000000..91df8ab6399727
--- /dev/null
+++ b/llvm/lib/Frontend/HLSL/HLSLRootSignature.cpp
@@ -0,0 +1,77 @@
+//===- HLSLRootSignature.cpp - HLSL Root Signature helper objects ----------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+///
+/// \file This file contains helpers for working with HLSL Root Signatures.
+///
+//===----------------------------------------------------------------------===//
+
+#include "llvm/Frontend/HLSL/HLSLRootSignature.h"
+#include "llvm/IR/IRBuilder.h"
+#include "llvm/IR/Metadata.h"
+#include "llvm/IR/Module.h"
+
+namespace llvm {
+namespace hlsl {
+namespace root_signature {
+
+static MDString *ClauseTypeToName(LLVMContext &Ctx, ClauseType Type) {
+  StringRef Name;
+  switch (Type) {
+  case ClauseType::CBuffer:
+    Name = "CBV";
+    break;
+  case ClauseType::SRV:
+    Name = "SRV";
+    break;
+  case ClauseType::UAV:
+    Name = "UAV";
+    break;
+  case ClauseType::Sampler:
+    Name = "Sampler";
+    break;
+  }
+  return MDString::get(Ctx, Name);
+}
+
+// Helper struct so that we can use the overloaded notation of std::visit
+template <class... Ts> struct OverloadBuilds : Ts... {
+  using Ts::operator()...;
+};
+template <class... Ts> OverloadBuilds(Ts...) -> OverloadBuilds<Ts...>;
+
+MDNode *MetadataBuilder::BuildRootSignature() {
+  for (const RootElement &Element : Elements) {
+    MDNode *ElementMD =
+      std::visit(
+        OverloadBuilds{
+            [&](DescriptorTable Table) -> MDNode * {
+              return BuildDescriptorTable(Table);
+            },
+            [&](DescriptorTableClause Clause) -> MDNode * {
+              return BuildDescriptorTableClause(Clause);
+            },
+        },
+        Element);
+    GeneratedMetadata.push_back(ElementMD);
+  }
+
+  return MDNode::get(Ctx, GeneratedMetadata);
+}
+
+MDNode *MetadataBuilder::BuildDescriptorTable(const DescriptorTable &Table) {
+  return MDNode::get(Ctx, {MDString::get(Ctx, "DescriptorTable")});
+}
+
+MDNode *MetadataBuilder::BuildDescriptorTableClause(const DescriptorTableClause &Clause) {
+  return MDNode::get(Ctx, {ClauseTypeToName(Ctx, Clause.Type)});
+}
+
+} // namespace root_signature
+} // namespace hlsl
+} // namespace llvm
+

>From afbf12094823f01c641dd8e1a942454d6c60a058 Mon Sep 17 00:00:00 2001
From: Finn Plummer <canadienfinn at gmail.com>
Date: Thu, 30 Jan 2025 22:45:58 +0000
Subject: [PATCH 4/7] add support for DescriptorTableClauses

---
 clang/test/CodeGenHLSL/RootSignature.hlsl    |  2 ++
 llvm/lib/Frontend/HLSL/HLSLRootSignature.cpp | 10 +++++++++-
 2 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/clang/test/CodeGenHLSL/RootSignature.hlsl b/clang/test/CodeGenHLSL/RootSignature.hlsl
index 659631133ad9b5..da8f2e5e24df14 100644
--- a/clang/test/CodeGenHLSL/RootSignature.hlsl
+++ b/clang/test/CodeGenHLSL/RootSignature.hlsl
@@ -5,11 +5,13 @@
 [numthreads(1,1,1)]
 void FirstEntry() {}
 
+// CHECK-DAG: ![[#CBV:]] = !{!"CBV", i32 1, i32 0, i32 0, i32 -1, i32 4}
 // CHECK-DAG: ![[#TABLE:]] = !{!"DescriptorTable"}
 // CHECK-DAG: ![[#SECOND_RS:]] = !{![[#TABLE]]}
 
 #define SampleDescriptorTable \
   "DescriptorTable( " \
+  "  CBV(b0) " \
   ")"
 [shader("compute"), RootSignature(SampleDescriptorTable)]
 [numthreads(1,1,1)]
diff --git a/llvm/lib/Frontend/HLSL/HLSLRootSignature.cpp b/llvm/lib/Frontend/HLSL/HLSLRootSignature.cpp
index 91df8ab6399727..0dbb3f0410a885 100644
--- a/llvm/lib/Frontend/HLSL/HLSLRootSignature.cpp
+++ b/llvm/lib/Frontend/HLSL/HLSLRootSignature.cpp
@@ -68,7 +68,15 @@ MDNode *MetadataBuilder::BuildDescriptorTable(const DescriptorTable &Table) {
 }
 
 MDNode *MetadataBuilder::BuildDescriptorTableClause(const DescriptorTableClause &Clause) {
-  return MDNode::get(Ctx, {ClauseTypeToName(Ctx, Clause.Type)});
+  IRBuilder<> B(Ctx);
+  return MDNode::get(Ctx, {
+    ClauseTypeToName(Ctx, Clause.Type),
+    ConstantAsMetadata::get(B.getInt32(Clause.NumDescriptors)),
+    ConstantAsMetadata::get(B.getInt32(Clause.Register.Number)),
+    ConstantAsMetadata::get(B.getInt32(Clause.Space)),
+    ConstantAsMetadata::get(B.getInt32(llvm::to_underlying(Clause.Offset))),
+    ConstantAsMetadata::get(B.getInt32(llvm::to_underlying(Clause.Flags))),
+  });
 }
 
 } // namespace root_signature

>From 016f380a03875fc1251794e6d4e9555d0c60d3e9 Mon Sep 17 00:00:00 2001
From: Finn Plummer <canadienfinn at gmail.com>
Date: Thu, 30 Jan 2025 22:51:42 +0000
Subject: [PATCH 5/7] add support for associating DescriptorTables with their
 clauses

---
 clang/test/CodeGenHLSL/RootSignature.hlsl    |  6 ++++--
 llvm/lib/Frontend/HLSL/HLSLRootSignature.cpp | 11 ++++++++++-
 2 files changed, 14 insertions(+), 3 deletions(-)

diff --git a/clang/test/CodeGenHLSL/RootSignature.hlsl b/clang/test/CodeGenHLSL/RootSignature.hlsl
index da8f2e5e24df14..2bbf4bf09093e6 100644
--- a/clang/test/CodeGenHLSL/RootSignature.hlsl
+++ b/clang/test/CodeGenHLSL/RootSignature.hlsl
@@ -6,12 +6,14 @@
 void FirstEntry() {}
 
 // CHECK-DAG: ![[#CBV:]] = !{!"CBV", i32 1, i32 0, i32 0, i32 -1, i32 4}
-// CHECK-DAG: ![[#TABLE:]] = !{!"DescriptorTable"}
+// CHECK-DAG: ![[#SRV:]] = !{!"SRV", i32 4, i32 42, i32 3, i32 32, i32 0}
+// CHECK-DAG: ![[#TABLE:]] = !{!"DescriptorTable", i32 0, ![[#CBV]], ![[#SRV]]}
 // CHECK-DAG: ![[#SECOND_RS:]] = !{![[#TABLE]]}
 
 #define SampleDescriptorTable \
   "DescriptorTable( " \
-  "  CBV(b0) " \
+  "  CBV(b0), " \
+  "  SRV(t42, space = 3, offset = 32, numDescriptors = 4, flags = 0) " \
   ")"
 [shader("compute"), RootSignature(SampleDescriptorTable)]
 [numthreads(1,1,1)]
diff --git a/llvm/lib/Frontend/HLSL/HLSLRootSignature.cpp b/llvm/lib/Frontend/HLSL/HLSLRootSignature.cpp
index 0dbb3f0410a885..0e88046a834443 100644
--- a/llvm/lib/Frontend/HLSL/HLSLRootSignature.cpp
+++ b/llvm/lib/Frontend/HLSL/HLSLRootSignature.cpp
@@ -64,7 +64,16 @@ MDNode *MetadataBuilder::BuildRootSignature() {
 }
 
 MDNode *MetadataBuilder::BuildDescriptorTable(const DescriptorTable &Table) {
-  return MDNode::get(Ctx, {MDString::get(Ctx, "DescriptorTable")});
+  IRBuilder<> B(Ctx);
+  SmallVector<Metadata *> TableOperands;
+  TableOperands.push_back(MDString::get(Ctx, "DescriptorTable"));
+  TableOperands.push_back(ConstantAsMetadata::get(B.getInt32(llvm::to_underlying(Table.Visibility))));
+
+  assert(Table.NumClauses <= GeneratedMetadata.size() && "Table expected all owned clauses to be generated already");
+  TableOperands.append(GeneratedMetadata.end() - Table.NumClauses, GeneratedMetadata.end());
+  GeneratedMetadata.pop_back_n(Table.NumClauses);
+
+  return MDNode::get(Ctx, TableOperands);
 }
 
 MDNode *MetadataBuilder::BuildDescriptorTableClause(const DescriptorTableClause &Clause) {

>From 611ef1fa8af41d384de734f74c4fff40c96246f8 Mon Sep 17 00:00:00 2001
From: Finn Plummer <canadienfinn at gmail.com>
Date: Thu, 30 Jan 2025 23:14:00 +0000
Subject: [PATCH 6/7] clean up

---
 clang/lib/CodeGen/CGHLSLRuntime.cpp                 | 3 +--
 clang/test/CodeGenHLSL/RootSignature.hlsl           | 2 +-
 llvm/include/llvm/Frontend/HLSL/HLSLRootSignature.h | 5 ++++-
 llvm/lib/Frontend/HLSL/HLSLRootSignature.cpp        | 8 ++++++++
 4 files changed, 14 insertions(+), 4 deletions(-)

diff --git a/clang/lib/CodeGen/CGHLSLRuntime.cpp b/clang/lib/CodeGen/CGHLSLRuntime.cpp
index 168bda18eb720d..bf0dc0fa7178fb 100644
--- a/clang/lib/CodeGen/CGHLSLRuntime.cpp
+++ b/clang/lib/CodeGen/CGHLSLRuntime.cpp
@@ -471,10 +471,9 @@ void CGHLSLRuntime::emitEntryFunction(const FunctionDecl *FD,
 
   // Add and identify root signature to function, if applicable
   const AttrVec &Attrs = FD->getAttrs();
-  for (const Attr *Attr : Attrs) {
+  for (const Attr *Attr : Attrs)
     if (const auto *RSAttr = dyn_cast<HLSLRootSignatureAttr>(Attr))
       addRootSignature(RSAttr->getElements(), EntryFn, M);
-  }
 }
 
 void CGHLSLRuntime::setHLSLFunctionAttributes(const FunctionDecl *FD,
diff --git a/clang/test/CodeGenHLSL/RootSignature.hlsl b/clang/test/CodeGenHLSL/RootSignature.hlsl
index 2bbf4bf09093e6..76aa2690b96a72 100644
--- a/clang/test/CodeGenHLSL/RootSignature.hlsl
+++ b/clang/test/CodeGenHLSL/RootSignature.hlsl
@@ -19,7 +19,7 @@ void FirstEntry() {}
 [numthreads(1,1,1)]
 void SecondEntry() {}
 
-// Sanity test to ensure to root is added for this function
+// Sanity test to ensure no root is added for this function
 [shader("compute")]
 [numthreads(1,1,1)]
 void ThirdEntry() {}
diff --git a/llvm/include/llvm/Frontend/HLSL/HLSLRootSignature.h b/llvm/include/llvm/Frontend/HLSL/HLSLRootSignature.h
index 433cd9bc106f6d..b96a47cd5c88c3 100644
--- a/llvm/include/llvm/Frontend/HLSL/HLSLRootSignature.h
+++ b/llvm/include/llvm/Frontend/HLSL/HLSLRootSignature.h
@@ -133,7 +133,10 @@ class MetadataBuilder {
   MetadataBuilder(llvm::LLVMContext &Ctx, ArrayRef<RootElement> Elements)
    : Ctx(Ctx), Elements(Elements) {}
 
-  // Iterates through the elements and builds the respective nodes
+  // Iterates through the elements and dispatches onto the correct Build method
+  //
+  // Accumulates the root signature and returns the Metadata node that is just
+  // a list of all the elements
   MDNode *BuildRootSignature();
 
  private:
diff --git a/llvm/lib/Frontend/HLSL/HLSLRootSignature.cpp b/llvm/lib/Frontend/HLSL/HLSLRootSignature.cpp
index 0e88046a834443..58847f827abc30 100644
--- a/llvm/lib/Frontend/HLSL/HLSLRootSignature.cpp
+++ b/llvm/lib/Frontend/HLSL/HLSLRootSignature.cpp
@@ -19,6 +19,8 @@ namespace llvm {
 namespace hlsl {
 namespace root_signature {
 
+// Static helper functions
+
 static MDString *ClauseTypeToName(LLVMContext &Ctx, ClauseType Type) {
   StringRef Name;
   switch (Type) {
@@ -66,11 +68,17 @@ MDNode *MetadataBuilder::BuildRootSignature() {
 MDNode *MetadataBuilder::BuildDescriptorTable(const DescriptorTable &Table) {
   IRBuilder<> B(Ctx);
   SmallVector<Metadata *> TableOperands;
+  // Set the mandatory arguments
   TableOperands.push_back(MDString::get(Ctx, "DescriptorTable"));
   TableOperands.push_back(ConstantAsMetadata::get(B.getInt32(llvm::to_underlying(Table.Visibility))));
 
+  // Remaining operands are references to the table's clauses. The in-memory
+  // representation of the Root Elements created from parsing will ensure that
+  // the previous N elements are the clauses for this table.
   assert(Table.NumClauses <= GeneratedMetadata.size() && "Table expected all owned clauses to be generated already");
+  // So, add a refence to each clause to our operands
   TableOperands.append(GeneratedMetadata.end() - Table.NumClauses, GeneratedMetadata.end());
+  // Then, remove those clauses from the general list of Root Elements
   GeneratedMetadata.pop_back_n(Table.NumClauses);
 
   return MDNode::get(Ctx, TableOperands);

>From 039f49d452dcb9ad8d3b10db8244c563fc06ca40 Mon Sep 17 00:00:00 2001
From: Finn Plummer <canadienfinn at gmail.com>
Date: Thu, 30 Jan 2025 23:14:46 +0000
Subject: [PATCH 7/7] clang format

---
 clang/lib/CodeGen/CGHLSLRuntime.cpp           |  4 +-
 .../llvm/Frontend/HLSL/HLSLRootSignature.h    |  6 +--
 llvm/lib/Frontend/HLSL/HLSLRootSignature.cpp  | 54 ++++++++++---------
 3 files changed, 35 insertions(+), 29 deletions(-)

diff --git a/clang/lib/CodeGen/CGHLSLRuntime.cpp b/clang/lib/CodeGen/CGHLSLRuntime.cpp
index bf0dc0fa7178fb..55c8faaa8de365 100644
--- a/clang/lib/CodeGen/CGHLSLRuntime.cpp
+++ b/clang/lib/CodeGen/CGHLSLRuntime.cpp
@@ -126,8 +126,8 @@ void addRootSignature(
 
   llvm::hlsl::root_signature::MetadataBuilder Builder(Ctx, Elements);
   MDNode *RootSignature = Builder.BuildRootSignature();
-  MDNode *FnPairing = MDNode::get(Ctx, {ValueAsMetadata::get(Fn),
-                                        RootSignature});
+  MDNode *FnPairing =
+      MDNode::get(Ctx, {ValueAsMetadata::get(Fn), RootSignature});
 
   StringRef RootSignatureValKey = "dx.rootsignatures";
   auto *RootSignatureValMD = M.getOrInsertNamedMetadata(RootSignatureValKey);
diff --git a/llvm/include/llvm/Frontend/HLSL/HLSLRootSignature.h b/llvm/include/llvm/Frontend/HLSL/HLSLRootSignature.h
index b96a47cd5c88c3..2d0cb350fe6ed3 100644
--- a/llvm/include/llvm/Frontend/HLSL/HLSLRootSignature.h
+++ b/llvm/include/llvm/Frontend/HLSL/HLSLRootSignature.h
@@ -129,9 +129,9 @@ using ParamType = std::variant<uint32_t *, DescriptorRangeOffset *,
                                DescriptorRangeFlags *, ShaderVisibility *>;
 
 class MetadataBuilder {
- public:
+public:
   MetadataBuilder(llvm::LLVMContext &Ctx, ArrayRef<RootElement> Elements)
-   : Ctx(Ctx), Elements(Elements) {}
+      : Ctx(Ctx), Elements(Elements) {}
 
   // Iterates through the elements and dispatches onto the correct Build method
   //
@@ -139,7 +139,7 @@ class MetadataBuilder {
   // a list of all the elements
   MDNode *BuildRootSignature();
 
- private:
+private:
   // Define the various builders for the different metadata types
   MDNode *BuildDescriptorTable(const DescriptorTable &Table);
   MDNode *BuildDescriptorTableClause(const DescriptorTableClause &Clause);
diff --git a/llvm/lib/Frontend/HLSL/HLSLRootSignature.cpp b/llvm/lib/Frontend/HLSL/HLSLRootSignature.cpp
index 58847f827abc30..d6b7a90dded91f 100644
--- a/llvm/lib/Frontend/HLSL/HLSLRootSignature.cpp
+++ b/llvm/lib/Frontend/HLSL/HLSLRootSignature.cpp
@@ -1,4 +1,5 @@
-//===- HLSLRootSignature.cpp - HLSL Root Signature helper objects ----------===//
+//===- HLSLRootSignature.cpp - HLSL Root Signature helper objects
+//----------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
@@ -49,16 +50,15 @@ template <class... Ts> OverloadBuilds(Ts...) -> OverloadBuilds<Ts...>;
 MDNode *MetadataBuilder::BuildRootSignature() {
   for (const RootElement &Element : Elements) {
     MDNode *ElementMD =
-      std::visit(
-        OverloadBuilds{
-            [&](DescriptorTable Table) -> MDNode * {
-              return BuildDescriptorTable(Table);
-            },
-            [&](DescriptorTableClause Clause) -> MDNode * {
-              return BuildDescriptorTableClause(Clause);
-            },
-        },
-        Element);
+        std::visit(OverloadBuilds{
+                       [&](DescriptorTable Table) -> MDNode * {
+                         return BuildDescriptorTable(Table);
+                       },
+                       [&](DescriptorTableClause Clause) -> MDNode * {
+                         return BuildDescriptorTableClause(Clause);
+                       },
+                   },
+                   Element);
     GeneratedMetadata.push_back(ElementMD);
   }
 
@@ -70,33 +70,39 @@ MDNode *MetadataBuilder::BuildDescriptorTable(const DescriptorTable &Table) {
   SmallVector<Metadata *> TableOperands;
   // Set the mandatory arguments
   TableOperands.push_back(MDString::get(Ctx, "DescriptorTable"));
-  TableOperands.push_back(ConstantAsMetadata::get(B.getInt32(llvm::to_underlying(Table.Visibility))));
+  TableOperands.push_back(ConstantAsMetadata::get(
+      B.getInt32(llvm::to_underlying(Table.Visibility))));
 
   // Remaining operands are references to the table's clauses. The in-memory
   // representation of the Root Elements created from parsing will ensure that
   // the previous N elements are the clauses for this table.
-  assert(Table.NumClauses <= GeneratedMetadata.size() && "Table expected all owned clauses to be generated already");
+  assert(Table.NumClauses <= GeneratedMetadata.size() &&
+         "Table expected all owned clauses to be generated already");
   // So, add a refence to each clause to our operands
-  TableOperands.append(GeneratedMetadata.end() - Table.NumClauses, GeneratedMetadata.end());
+  TableOperands.append(GeneratedMetadata.end() - Table.NumClauses,
+                       GeneratedMetadata.end());
   // Then, remove those clauses from the general list of Root Elements
   GeneratedMetadata.pop_back_n(Table.NumClauses);
 
   return MDNode::get(Ctx, TableOperands);
 }
 
-MDNode *MetadataBuilder::BuildDescriptorTableClause(const DescriptorTableClause &Clause) {
+MDNode *MetadataBuilder::BuildDescriptorTableClause(
+    const DescriptorTableClause &Clause) {
   IRBuilder<> B(Ctx);
-  return MDNode::get(Ctx, {
-    ClauseTypeToName(Ctx, Clause.Type),
-    ConstantAsMetadata::get(B.getInt32(Clause.NumDescriptors)),
-    ConstantAsMetadata::get(B.getInt32(Clause.Register.Number)),
-    ConstantAsMetadata::get(B.getInt32(Clause.Space)),
-    ConstantAsMetadata::get(B.getInt32(llvm::to_underlying(Clause.Offset))),
-    ConstantAsMetadata::get(B.getInt32(llvm::to_underlying(Clause.Flags))),
-  });
+  return MDNode::get(
+      Ctx, {
+               ClauseTypeToName(Ctx, Clause.Type),
+               ConstantAsMetadata::get(B.getInt32(Clause.NumDescriptors)),
+               ConstantAsMetadata::get(B.getInt32(Clause.Register.Number)),
+               ConstantAsMetadata::get(B.getInt32(Clause.Space)),
+               ConstantAsMetadata::get(
+                   B.getInt32(llvm::to_underlying(Clause.Offset))),
+               ConstantAsMetadata::get(
+                   B.getInt32(llvm::to_underlying(Clause.Flags))),
+           });
 }
 
 } // namespace root_signature
 } // namespace hlsl
 } // namespace llvm
-



More information about the llvm-branch-commits mailing list