[HLSL][RootSignature] Add metadata generation for descriptor tables #139633
+164
−7
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- test that we have the correct number of elements
llvmbot
added
clang
|
@llvm/pr-subscribers-clang @llvm/pr-subscribers-hlsl Author: Finn Plummer (inbelic) Changes
Resolves #126584 Note: this is essentially just #125131 rebased onto the new approach of constructing a root signature decl, instead of holding the elements in Full diff: https://github.com/llvm/llvm-project/pull/139633.diff 6 Files Affected:
diff --git a/clang/include/clang/Basic/Attr.td b/clang/include/clang/Basic/Attr.td
index ccd13a4cca4dd..dda62fdf03586 100644
--- a/clang/include/clang/Basic/Attr.td
+++ b/clang/include/clang/Basic/Attr.td
@@ -4739,7 +4739,8 @@ def Error : InheritableAttr {
def RootSignature : Attr {
/// [RootSignature(Signature)]
let Spellings = [Microsoft<"RootSignature">];
- let Args = [IdentifierArgument<"Signature">];
+ let Args = [IdentifierArgument<"SignatureIdent">,
+ DeclArgument<HLSLRootSignature, "SignatureDecl", 0, /*fake=*/1>];
let Subjects = SubjectList<[Function],
ErrorDiag, "'function'">;
let LangOpts = [HLSL];
diff --git a/clang/lib/CodeGen/CGHLSLRuntime.cpp b/clang/lib/CodeGen/CGHLSLRuntime.cpp
index 0eb4bb062e02e..61fc65863a12e 100644
--- a/clang/lib/CodeGen/CGHLSLRuntime.cpp
+++ b/clang/lib/CodeGen/CGHLSLRuntime.cpp
@@ -68,6 +68,20 @@ void addDxilValVersion(StringRef ValVersionStr, llvm::Module &M) {
DXILValMD->addOperand(Val);
}
+void addRootSignature(ArrayRef<llvm::hlsl::rootsig::RootElement> Elements,
+ llvm::Function *Fn, llvm::Module &M) {
+ auto &Ctx = M.getContext();
+
+ llvm::hlsl::rootsig::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(FnPairing);
+}
+
} // namespace
llvm::Type *
@@ -423,6 +437,14 @@ 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 (const auto *RSAttr = dyn_cast<RootSignatureAttr>(Attr))
+ addRootSignature(RSAttr->getSignatureDecl()->getRootElements(), EntryFn,
+ M);
+ }
}
void CGHLSLRuntime::setHLSLFunctionAttributes(const FunctionDecl *FD,
diff --git a/clang/lib/Sema/SemaHLSL.cpp b/clang/lib/Sema/SemaHLSL.cpp
index 744ec439b2393..1b1b6573b7af3 100644
--- a/clang/lib/Sema/SemaHLSL.cpp
+++ b/clang/lib/Sema/SemaHLSL.cpp
@@ -959,7 +959,7 @@ void SemaHLSL::handleRootSignatureAttr(Decl *D, const ParsedAttr &AL) {
IdentifierInfo *Ident = AL.getArgAsIdent(0)->getIdentifierInfo();
if (auto *RS = D->getAttr<RootSignatureAttr>()) {
- if (RS->getSignature() != Ident) {
+ if (RS->getSignatureIdent() != Ident) {
Diag(AL.getLoc(), diag::err_disallowed_duplicate_attribute) << RS;
return;
}
@@ -970,10 +970,11 @@ void SemaHLSL::handleRootSignatureAttr(Decl *D, const ParsedAttr &AL) {
LookupResult R(SemaRef, Ident, SourceLocation(), Sema::LookupOrdinaryName);
if (SemaRef.LookupQualifiedName(R, D->getDeclContext()))
- if (isa<HLSLRootSignatureDecl>(R.getFoundDecl())) {
+ if (auto *SignatureDecl =
+ dyn_cast<HLSLRootSignatureDecl>(R.getFoundDecl())) {
// Perform validation of constructs here
- D->addAttr(::new (getASTContext())
- RootSignatureAttr(getASTContext(), AL, Ident));
+ D->addAttr(::new (getASTContext()) RootSignatureAttr(
+ getASTContext(), AL, Ident, SignatureDecl));
}
}
diff --git a/clang/test/CodeGenHLSL/RootSignature.hlsl b/clang/test/CodeGenHLSL/RootSignature.hlsl
new file mode 100644
index 0000000000000..60e0dec175b8f
--- /dev/null
+++ b/clang/test/CodeGenHLSL/RootSignature.hlsl
@@ -0,0 +1,31 @@
+// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.3-library -emit-llvm -o - %s | FileCheck %s
+
+// CHECK: !dx.rootsignatures = !{![[#FIRST_ENTRY:]], ![[#SECOND_ENTRY:]]}
+
+// CHECK: ![[#FIRST_ENTRY]] = !{ptr @FirstEntry, ![[#EMPTY:]]}
+// CHECK: ![[#EMPTY]] = !{}
+
+[shader("compute"), RootSignature("")]
+[numthreads(1,1,1)]
+void FirstEntry() {}
+
+// CHECK: ![[#SECOND_ENTRY]] = !{ptr @SecondEntry, ![[#SECOND_RS:]]}
+// CHECK: ![[#SECOND_RS]] = !{![[#TABLE:]]}
+// CHECK: ![[#TABLE]] = !{!"DescriptorTable", i32 0, ![[#CBV:]], ![[#SRV:]]}
+// CHECK: ![[#CBV]] = !{!"CBV", i32 1, i32 0, i32 0, i32 -1, i32 4}
+// CHECK: ![[#SRV]] = !{!"SRV", i32 4, i32 42, i32 3, i32 32, i32 0}
+
+#define SampleDescriptorTable \
+ "DescriptorTable( " \
+ " CBV(b0), " \
+ " SRV(t42, space = 3, offset = 32, numDescriptors = 4, flags = 0) " \
+ ")"
+[shader("compute"), RootSignature(SampleDescriptorTable)]
+[numthreads(1,1,1)]
+void SecondEntry() {}
+
+// Sanity test to ensure no root is added for this function as there is only
+// two entries in !dx.roosignatures
+[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 37f3d9ad61d3e..585a1338fa4d0 100644
--- a/llvm/include/llvm/Frontend/HLSL/HLSLRootSignature.h
+++ b/llvm/include/llvm/Frontend/HLSL/HLSLRootSignature.h
@@ -15,11 +15,16 @@
#define LLVM_FRONTEND_HLSL_HLSLROOTSIGNATURE_H
#include "llvm/ADT/ArrayRef.h"
+// #include "llvm/ADT/STLForwardCompat.h"
#include "llvm/Support/DXILABI.h"
#include "llvm/Support/raw_ostream.h"
#include <variant>
namespace llvm {
+class LLVMContext;
+class MDNode;
+class Metadata;
+
namespace hlsl {
namespace rootsig {
@@ -125,6 +130,27 @@ using RootElement = std::variant<RootFlags, RootConstants, DescriptorTable,
void dumpRootElements(raw_ostream &OS, ArrayRef<RootElement> Elements);
+class MetadataBuilder {
+public:
+ MetadataBuilder(llvm::LLVMContext &Ctx, ArrayRef<RootElement> Elements)
+ : Ctx(Ctx), Elements(Elements) {}
+
+ /// 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:
+ /// 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 rootsig
} // namespace hlsl
} // namespace llvm
diff --git a/llvm/lib/Frontend/HLSL/HLSLRootSignature.cpp b/llvm/lib/Frontend/HLSL/HLSLRootSignature.cpp
index cd3c6f8dde8be..301de33400349 100644
--- a/llvm/lib/Frontend/HLSL/HLSLRootSignature.cpp
+++ b/llvm/lib/Frontend/HLSL/HLSLRootSignature.cpp
@@ -12,6 +12,9 @@
#include "llvm/Frontend/HLSL/HLSLRootSignature.h"
#include "llvm/ADT/bit.h"
+#include "llvm/IR/IRBuilder.h"
+#include "llvm/IR/Metadata.h"
+#include "llvm/IR/Module.h"
namespace llvm {
namespace hlsl {
@@ -160,6 +163,75 @@ void dumpRootElements(raw_ostream &OS, ArrayRef<RootElement> Elements) {
OS << "}";
}
+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);
+}
+
+MDNode *MetadataBuilder::BuildRootSignature() {
+ for (const RootElement &Element : Elements) {
+ MDNode *ElementMD = nullptr;
+ if (const auto &Clause = std::get_if<DescriptorTableClause>(&Element))
+ ElementMD = BuildDescriptorTableClause(*Clause);
+ if (const auto &Table = std::get_if<DescriptorTable>(&Element))
+ ElementMD = BuildDescriptorTable(*Table);
+ GeneratedMetadata.push_back(ElementMD);
+ }
+
+ return MDNode::get(Ctx, GeneratedMetadata);
+}
+
+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);
+}
+
+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.Reg.Number)),
+ ConstantAsMetadata::get(B.getInt32(Clause.Space)),
+ ConstantAsMetadata::get(B.getInt32(Clause.Offset)),
+ ConstantAsMetadata::get(
+ B.getInt32(llvm::to_underlying(Clause.Flags))),
+ });
+}
+
} // namespace rootsig
} // namespace hlsl
} // namespace llvm
|
|
@llvm/pr-subscribers-clang-codegen Author: Finn Plummer (inbelic) Changes
Resolves #126584 Note: this is essentially just #125131 rebased onto the new approach of constructing a root signature decl, instead of holding the elements in Full diff: https://github.com/llvm/llvm-project/pull/139633.diff 6 Files Affected:
diff --git a/clang/include/clang/Basic/Attr.td b/clang/include/clang/Basic/Attr.td
index ccd13a4cca4dd..dda62fdf03586 100644
--- a/clang/include/clang/Basic/Attr.td
+++ b/clang/include/clang/Basic/Attr.td
@@ -4739,7 +4739,8 @@ def Error : InheritableAttr {
def RootSignature : Attr {
/// [RootSignature(Signature)]
let Spellings = [Microsoft<"RootSignature">];
- let Args = [IdentifierArgument<"Signature">];
+ let Args = [IdentifierArgument<"SignatureIdent">,
+ DeclArgument<HLSLRootSignature, "SignatureDecl", 0, /*fake=*/1>];
let Subjects = SubjectList<[Function],
ErrorDiag, "'function'">;
let LangOpts = [HLSL];
diff --git a/clang/lib/CodeGen/CGHLSLRuntime.cpp b/clang/lib/CodeGen/CGHLSLRuntime.cpp
index 0eb4bb062e02e..61fc65863a12e 100644
--- a/clang/lib/CodeGen/CGHLSLRuntime.cpp
+++ b/clang/lib/CodeGen/CGHLSLRuntime.cpp
@@ -68,6 +68,20 @@ void addDxilValVersion(StringRef ValVersionStr, llvm::Module &M) {
DXILValMD->addOperand(Val);
}
+void addRootSignature(ArrayRef<llvm::hlsl::rootsig::RootElement> Elements,
+ llvm::Function *Fn, llvm::Module &M) {
+ auto &Ctx = M.getContext();
+
+ llvm::hlsl::rootsig::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(FnPairing);
+}
+
} // namespace
llvm::Type *
@@ -423,6 +437,14 @@ 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 (const auto *RSAttr = dyn_cast<RootSignatureAttr>(Attr))
+ addRootSignature(RSAttr->getSignatureDecl()->getRootElements(), EntryFn,
+ M);
+ }
}
void CGHLSLRuntime::setHLSLFunctionAttributes(const FunctionDecl *FD,
diff --git a/clang/lib/Sema/SemaHLSL.cpp b/clang/lib/Sema/SemaHLSL.cpp
index 744ec439b2393..1b1b6573b7af3 100644
--- a/clang/lib/Sema/SemaHLSL.cpp
+++ b/clang/lib/Sema/SemaHLSL.cpp
@@ -959,7 +959,7 @@ void SemaHLSL::handleRootSignatureAttr(Decl *D, const ParsedAttr &AL) {
IdentifierInfo *Ident = AL.getArgAsIdent(0)->getIdentifierInfo();
if (auto *RS = D->getAttr<RootSignatureAttr>()) {
- if (RS->getSignature() != Ident) {
+ if (RS->getSignatureIdent() != Ident) {
Diag(AL.getLoc(), diag::err_disallowed_duplicate_attribute) << RS;
return;
}
@@ -970,10 +970,11 @@ void SemaHLSL::handleRootSignatureAttr(Decl *D, const ParsedAttr &AL) {
LookupResult R(SemaRef, Ident, SourceLocation(), Sema::LookupOrdinaryName);
if (SemaRef.LookupQualifiedName(R, D->getDeclContext()))
- if (isa<HLSLRootSignatureDecl>(R.getFoundDecl())) {
+ if (auto *SignatureDecl =
+ dyn_cast<HLSLRootSignatureDecl>(R.getFoundDecl())) {
// Perform validation of constructs here
- D->addAttr(::new (getASTContext())
- RootSignatureAttr(getASTContext(), AL, Ident));
+ D->addAttr(::new (getASTContext()) RootSignatureAttr(
+ getASTContext(), AL, Ident, SignatureDecl));
}
}
diff --git a/clang/test/CodeGenHLSL/RootSignature.hlsl b/clang/test/CodeGenHLSL/RootSignature.hlsl
new file mode 100644
index 0000000000000..60e0dec175b8f
--- /dev/null
+++ b/clang/test/CodeGenHLSL/RootSignature.hlsl
@@ -0,0 +1,31 @@
+// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.3-library -emit-llvm -o - %s | FileCheck %s
+
+// CHECK: !dx.rootsignatures = !{![[#FIRST_ENTRY:]], ![[#SECOND_ENTRY:]]}
+
+// CHECK: ![[#FIRST_ENTRY]] = !{ptr @FirstEntry, ![[#EMPTY:]]}
+// CHECK: ![[#EMPTY]] = !{}
+
+[shader("compute"), RootSignature("")]
+[numthreads(1,1,1)]
+void FirstEntry() {}
+
+// CHECK: ![[#SECOND_ENTRY]] = !{ptr @SecondEntry, ![[#SECOND_RS:]]}
+// CHECK: ![[#SECOND_RS]] = !{![[#TABLE:]]}
+// CHECK: ![[#TABLE]] = !{!"DescriptorTable", i32 0, ![[#CBV:]], ![[#SRV:]]}
+// CHECK: ![[#CBV]] = !{!"CBV", i32 1, i32 0, i32 0, i32 -1, i32 4}
+// CHECK: ![[#SRV]] = !{!"SRV", i32 4, i32 42, i32 3, i32 32, i32 0}
+
+#define SampleDescriptorTable \
+ "DescriptorTable( " \
+ " CBV(b0), " \
+ " SRV(t42, space = 3, offset = 32, numDescriptors = 4, flags = 0) " \
+ ")"
+[shader("compute"), RootSignature(SampleDescriptorTable)]
+[numthreads(1,1,1)]
+void SecondEntry() {}
+
+// Sanity test to ensure no root is added for this function as there is only
+// two entries in !dx.roosignatures
+[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 37f3d9ad61d3e..585a1338fa4d0 100644
--- a/llvm/include/llvm/Frontend/HLSL/HLSLRootSignature.h
+++ b/llvm/include/llvm/Frontend/HLSL/HLSLRootSignature.h
@@ -15,11 +15,16 @@
#define LLVM_FRONTEND_HLSL_HLSLROOTSIGNATURE_H
#include "llvm/ADT/ArrayRef.h"
+// #include "llvm/ADT/STLForwardCompat.h"
#include "llvm/Support/DXILABI.h"
#include "llvm/Support/raw_ostream.h"
#include <variant>
namespace llvm {
+class LLVMContext;
+class MDNode;
+class Metadata;
+
namespace hlsl {
namespace rootsig {
@@ -125,6 +130,27 @@ using RootElement = std::variant<RootFlags, RootConstants, DescriptorTable,
void dumpRootElements(raw_ostream &OS, ArrayRef<RootElement> Elements);
+class MetadataBuilder {
+public:
+ MetadataBuilder(llvm::LLVMContext &Ctx, ArrayRef<RootElement> Elements)
+ : Ctx(Ctx), Elements(Elements) {}
+
+ /// 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:
+ /// 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 rootsig
} // namespace hlsl
} // namespace llvm
diff --git a/llvm/lib/Frontend/HLSL/HLSLRootSignature.cpp b/llvm/lib/Frontend/HLSL/HLSLRootSignature.cpp
index cd3c6f8dde8be..301de33400349 100644
--- a/llvm/lib/Frontend/HLSL/HLSLRootSignature.cpp
+++ b/llvm/lib/Frontend/HLSL/HLSLRootSignature.cpp
@@ -12,6 +12,9 @@
#include "llvm/Frontend/HLSL/HLSLRootSignature.h"
#include "llvm/ADT/bit.h"
+#include "llvm/IR/IRBuilder.h"
+#include "llvm/IR/Metadata.h"
+#include "llvm/IR/Module.h"
namespace llvm {
namespace hlsl {
@@ -160,6 +163,75 @@ void dumpRootElements(raw_ostream &OS, ArrayRef<RootElement> Elements) {
OS << "}";
}
+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);
+}
+
+MDNode *MetadataBuilder::BuildRootSignature() {
+ for (const RootElement &Element : Elements) {
+ MDNode *ElementMD = nullptr;
+ if (const auto &Clause = std::get_if<DescriptorTableClause>(&Element))
+ ElementMD = BuildDescriptorTableClause(*Clause);
+ if (const auto &Table = std::get_if<DescriptorTable>(&Element))
+ ElementMD = BuildDescriptorTable(*Table);
+ GeneratedMetadata.push_back(ElementMD);
+ }
+
+ return MDNode::get(Ctx, GeneratedMetadata);
+}
+
+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);
+}
+
+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.Reg.Number)),
+ ConstantAsMetadata::get(B.getInt32(Clause.Space)),
+ ConstantAsMetadata::get(B.getInt32(Clause.Offset)),
+ ConstantAsMetadata::get(
+ B.getInt32(llvm::to_underlying(Clause.Flags))),
+ });
+}
+
} // namespace rootsig
} // namespace hlsl
} // namespace llvm
|
inbelic
commented
May 12, 2025
joaosaffran
reviewed
May 13, 2025
Comment on lines
+209
to
+210
| assert(Table.NumClauses <= GeneratedMetadata.size() && | ||
| "Table expected all owned clauses to be generated already"); |
There was a problem hiding this comment.
Not sure if that assert would really help, since it could easily be mislead by the generation of other parameters.
There was a problem hiding this comment.
This was added as a (hopefully) better error message rather than the error that would be generated if Generated.size() - Table.NumClauses created an undefined behaviour iterator.
Do you have a suggestion for how it could be better worded, or, a better assert?
There was a problem hiding this comment.
Not sure how complex/useful this is, we could check how many DescriptorTableClause are in TableOperands preceding the table. I think this could be done like:
uint Count = 0;
// I think this should be the Element parameter, from BuildRootSignature
auto It = Element;
while (std::holds_alternative<DescriptorTableClause>(*It)){
Count ++;
Element --;
}
assert(Table.NumClauses == Count);Not sure this code is correct/useful, but that is the idea.
There was a problem hiding this comment.
I like the idea and toyed around with it a bit. It unfortunately is a bit a unwieldly to implement. I think it introduces more complexity then it reduces
- formally document the structure of the in-memory representation of a root signature as an array of the defined RootElements
bogner
approved these changes
May 15, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
RootSignatureAttrto hold a reference to the owned declarationMetadataBuilderinHLSLRootSignatureCGHLSLRuntime.cppto generate the Root Signature for any associated entry functionsRootSignature.hlslResolves #126584
Note: this is essentially just #125131 rebased onto the new approach of constructing a root signature decl, instead of holding the elements in
AdditionalMembers.