https://github.com/inbelic updated https://github.com/llvm/llvm-project/pull/137690
>From 87680ba4a99d35dbcc9416ab6b0710bd2f88d7bd Mon Sep 17 00:00:00 2001 From: Finn Plummer <canadienf...@gmail.com> Date: Mon, 28 Apr 2025 18:42:10 +0000 Subject: [PATCH 01/16] [HLSL][RootSignature] Define and integrate rootsig clang attr and decl - Defines a new declaration node `HLSLRootSignature` in `DeclNodes.td` that will hold a reference to an in-memory construction of the root signature, namely an array of `hlsl::rootsig::RootElement`s - Defines a new clang attr `RootSignature` which simply holds an identifier to a corresponding root signature declration as above It was previously proposed that we could have the root elements reference be stored directly as an additional member of the attribute and to not have a seperate root signature decl. In contrast, by defining them seperately as this change proposes, we will allow a unique root signature to have its own declaration in the AST tree. This allows us to only construct a single root signature for all duplicate rootsignature attributes. Having it located directly as a declaration might also prove advantageous when we consider root signature libraries. --- clang/include/clang/AST/Decl.h | 24 +++++ clang/include/clang/AST/RecursiveASTVisitor.h | 2 + clang/include/clang/AST/TextNodeDumper.h | 1 + clang/include/clang/Basic/Attr.td | 11 +++ clang/include/clang/Basic/AttrDocs.td | 11 +++ clang/include/clang/Basic/DeclNodes.td | 1 + clang/include/clang/Parse/Parser.h | 1 + clang/include/clang/Sema/SemaHLSL.h | 1 + clang/lib/AST/Decl.cpp | 25 ++++++ clang/lib/AST/DeclBase.cpp | 1 + clang/lib/AST/TextNodeDumper.cpp | 4 + clang/lib/CodeGen/CGDecl.cpp | 1 + clang/lib/Parse/ParseDeclCXX.cpp | 90 +++++++++++++++++++ clang/lib/Sema/SemaDeclAttr.cpp | 3 + clang/lib/Sema/SemaHLSL.cpp | 18 ++++ .../lib/Sema/SemaTemplateInstantiateDecl.cpp | 5 ++ clang/lib/Serialization/ASTCommon.cpp | 1 + clang/test/AST/HLSL/RootSignatures-AST.hlsl | 58 ++++++++++++ clang/test/SemaHLSL/RootSignature-err.hlsl | 11 +++ clang/tools/libclang/CIndex.cpp | 1 + 20 files changed, 270 insertions(+) create mode 100644 clang/test/AST/HLSL/RootSignatures-AST.hlsl create mode 100644 clang/test/SemaHLSL/RootSignature-err.hlsl diff --git a/clang/include/clang/AST/Decl.h b/clang/include/clang/AST/Decl.h index 3faf63e395a08..8e45c19061b1d 100644 --- a/clang/include/clang/AST/Decl.h +++ b/clang/include/clang/AST/Decl.h @@ -41,6 +41,7 @@ #include "llvm/ADT/PointerUnion.h" #include "llvm/ADT/StringRef.h" #include "llvm/ADT/iterator_range.h" +#include "llvm/Frontend/HLSL/HLSLRootSignature.h" #include "llvm/Support/Casting.h" #include "llvm/Support/Compiler.h" #include "llvm/Support/TrailingObjects.h" @@ -5178,6 +5179,29 @@ class HLSLBufferDecl final : public NamedDecl, public DeclContext { friend class ASTDeclWriter; }; +class HLSLRootSignatureDecl final : public NamedDecl { + ArrayRef<llvm::hlsl::rootsig::RootElement> RootElements; + + HLSLRootSignatureDecl( + DeclContext *DC, SourceLocation Loc, IdentifierInfo *ID, + ArrayRef<llvm::hlsl::rootsig::RootElement> RootElements); + +public: + static HLSLRootSignatureDecl * + Create(ASTContext &C, DeclContext *DC, SourceLocation Loc, IdentifierInfo *ID, + ArrayRef<llvm::hlsl::rootsig::RootElement> RootElements); + static HLSLRootSignatureDecl *CreateDeserialized(ASTContext &C, + GlobalDeclID ID); + + ArrayRef<llvm::hlsl::rootsig::RootElement> &getRootElements() { + return RootElements; + } + + // Implement isa/cast/dyncast/etc. + static bool classof(const Decl *D) { return classofKind(D->getKind()); } + static bool classofKind(Kind K) { return K == HLSLRootSignature; } +}; + /// Insertion operator for diagnostics. This allows sending NamedDecl's /// into a diagnostic with <<. inline const StreamingDiagnostic &operator<<(const StreamingDiagnostic &PD, diff --git a/clang/include/clang/AST/RecursiveASTVisitor.h b/clang/include/clang/AST/RecursiveASTVisitor.h index 3edc8684d0a19..23a8c4f1f7380 100644 --- a/clang/include/clang/AST/RecursiveASTVisitor.h +++ b/clang/include/clang/AST/RecursiveASTVisitor.h @@ -1599,6 +1599,8 @@ DEF_TRAVERSE_DECL(EmptyDecl, {}) DEF_TRAVERSE_DECL(HLSLBufferDecl, {}) +DEF_TRAVERSE_DECL(HLSLRootSignatureDecl, {}) + DEF_TRAVERSE_DECL(LifetimeExtendedTemporaryDecl, { TRY_TO(TraverseStmt(D->getTemporaryExpr())); }) diff --git a/clang/include/clang/AST/TextNodeDumper.h b/clang/include/clang/AST/TextNodeDumper.h index ea3a0f058a8ed..1917a8ac29f05 100644 --- a/clang/include/clang/AST/TextNodeDumper.h +++ b/clang/include/clang/AST/TextNodeDumper.h @@ -408,6 +408,7 @@ class TextNodeDumper void VisitLifetimeExtendedTemporaryDecl(const LifetimeExtendedTemporaryDecl *D); void VisitHLSLBufferDecl(const HLSLBufferDecl *D); + void VisitHLSLRootSignatureDecl(const HLSLRootSignatureDecl *D); void VisitHLSLOutArgExpr(const HLSLOutArgExpr *E); void VisitOpenACCConstructStmt(const OpenACCConstructStmt *S); void VisitOpenACCLoopConstruct(const OpenACCLoopConstruct *S); diff --git a/clang/include/clang/Basic/Attr.td b/clang/include/clang/Basic/Attr.td index 37c80ac90182c..ccd13a4cca4dd 100644 --- a/clang/include/clang/Basic/Attr.td +++ b/clang/include/clang/Basic/Attr.td @@ -4735,6 +4735,17 @@ def Error : InheritableAttr { let Documentation = [ErrorAttrDocs]; } +/// HLSL Root Signature Attribute +def RootSignature : Attr { + /// [RootSignature(Signature)] + let Spellings = [Microsoft<"RootSignature">]; + let Args = [IdentifierArgument<"Signature">]; + let Subjects = SubjectList<[Function], + ErrorDiag, "'function'">; + let LangOpts = [HLSL]; + let Documentation = [RootSignatureDocs]; +} + def HLSLNumThreads: InheritableAttr { let Spellings = [Microsoft<"numthreads">]; let Args = [IntArgument<"X">, IntArgument<"Y">, IntArgument<"Z">]; diff --git a/clang/include/clang/Basic/AttrDocs.td b/clang/include/clang/Basic/AttrDocs.td index cbb397cb31dfb..5fb5f16680b41 100644 --- a/clang/include/clang/Basic/AttrDocs.td +++ b/clang/include/clang/Basic/AttrDocs.td @@ -8195,6 +8195,17 @@ and https://microsoft.github.io/hlsl-specs/proposals/0013-wave-size-range.html }]; } +def RootSignatureDocs : Documentation { + let Category = DocCatFunction; + let Content = [{ +The ``RootSignature`` attribute applies to HLSL entry functions to define what +types of resources are bound to the graphics pipeline. + +For details about the use and specification of Root Signatures please see here: +https://learn.microsoft.com/en-us/windows/win32/direct3d12/root-signatures + }]; +} + def NumThreadsDocs : Documentation { let Category = DocCatFunction; let Content = [{ diff --git a/clang/include/clang/Basic/DeclNodes.td b/clang/include/clang/Basic/DeclNodes.td index 20debd67a31a5..f1ebaf1db3fc0 100644 --- a/clang/include/clang/Basic/DeclNodes.td +++ b/clang/include/clang/Basic/DeclNodes.td @@ -111,5 +111,6 @@ def Empty : DeclNode<Decl>; def RequiresExprBody : DeclNode<Decl>, DeclContext; def LifetimeExtendedTemporary : DeclNode<Decl>; def HLSLBuffer : DeclNode<Named, "HLSLBuffer">, DeclContext; +def HLSLRootSignature : DeclNode<Named, "HLSLRootSignature">; def OpenACCDeclare : DeclNode<Decl, "#pragma acc declare">; def OpenACCRoutine : DeclNode<Decl, "#pragma acc routine">; diff --git a/clang/include/clang/Parse/Parser.h b/clang/include/clang/Parse/Parser.h index d2fec2b7a6462..cab56f9edbcf1 100644 --- a/clang/include/clang/Parse/Parser.h +++ b/clang/include/clang/Parse/Parser.h @@ -3093,6 +3093,7 @@ class Parser : public CodeCompletionHandler { return AttrsParsed; } void ParseMicrosoftUuidAttributeArgs(ParsedAttributes &Attrs); + void ParseMicrosoftRootSignatureAttributeArgs(ParsedAttributes &Attrs); void ParseMicrosoftAttributes(ParsedAttributes &Attrs); bool MaybeParseMicrosoftDeclSpecs(ParsedAttributes &Attrs) { if (getLangOpts().DeclSpecKeyword && Tok.is(tok::kw___declspec)) { diff --git a/clang/include/clang/Sema/SemaHLSL.h b/clang/include/clang/Sema/SemaHLSL.h index 5d260acf92abb..e340547ff5f45 100644 --- a/clang/include/clang/Sema/SemaHLSL.h +++ b/clang/include/clang/Sema/SemaHLSL.h @@ -119,6 +119,7 @@ class SemaHLSL : public SemaBase { bool IsCompAssign); void emitLogicalOperatorFixIt(Expr *LHS, Expr *RHS, BinaryOperatorKind Opc); + void handleRootSignatureAttr(Decl *D, const ParsedAttr &AL); void handleNumThreadsAttr(Decl *D, const ParsedAttr &AL); void handleWaveSizeAttr(Decl *D, const ParsedAttr &AL); void handleSV_DispatchThreadIDAttr(Decl *D, const ParsedAttr &AL); diff --git a/clang/lib/AST/Decl.cpp b/clang/lib/AST/Decl.cpp index 9cd1c71afd0f8..8bf881a1b5f43 100644 --- a/clang/lib/AST/Decl.cpp +++ b/clang/lib/AST/Decl.cpp @@ -5847,6 +5847,31 @@ bool HLSLBufferDecl::buffer_decls_empty() { return DefaultBufferDecls.empty() && decls_empty(); } +//===----------------------------------------------------------------------===// +// HLSLRootSignatureDecl Implementation +//===----------------------------------------------------------------------===// + +HLSLRootSignatureDecl::HLSLRootSignatureDecl( + DeclContext *DC, SourceLocation Loc, IdentifierInfo *ID, + ArrayRef<llvm::hlsl::rootsig::RootElement> RootElements) + : NamedDecl(Decl::Kind::HLSLRootSignature, DC, Loc, DeclarationName(ID)), + RootElements(RootElements) {} + +HLSLRootSignatureDecl *HLSLRootSignatureDecl::Create( + ASTContext &C, DeclContext *DC, SourceLocation Loc, IdentifierInfo *ID, + ArrayRef<llvm::hlsl::rootsig::RootElement> RootElements) { + HLSLRootSignatureDecl *Result = + new (C, DC) HLSLRootSignatureDecl(DC, Loc, ID, RootElements); + return Result; +} + +HLSLRootSignatureDecl * +HLSLRootSignatureDecl::CreateDeserialized(ASTContext &C, GlobalDeclID ID) { + HLSLRootSignatureDecl *Result = + new (C, ID) HLSLRootSignatureDecl(nullptr, SourceLocation(), nullptr, {}); + return Result; +} + //===----------------------------------------------------------------------===// // ImportDecl Implementation //===----------------------------------------------------------------------===// diff --git a/clang/lib/AST/DeclBase.cpp b/clang/lib/AST/DeclBase.cpp index 2052c0c7cfe42..e30057e32d312 100644 --- a/clang/lib/AST/DeclBase.cpp +++ b/clang/lib/AST/DeclBase.cpp @@ -886,6 +886,7 @@ unsigned Decl::getIdentifierNamespaceForKind(Kind DeclKind) { case ObjCProperty: case MSProperty: case HLSLBuffer: + case HLSLRootSignature: return IDNS_Ordinary; case Label: return IDNS_Label; diff --git a/clang/lib/AST/TextNodeDumper.cpp b/clang/lib/AST/TextNodeDumper.cpp index 3af6276b4baa1..53364719369de 100644 --- a/clang/lib/AST/TextNodeDumper.cpp +++ b/clang/lib/AST/TextNodeDumper.cpp @@ -3037,6 +3037,10 @@ void TextNodeDumper::VisitHLSLBufferDecl(const HLSLBufferDecl *D) { dumpName(D); } +void TextNodeDumper::VisitHLSLRootSignatureDecl(const HLSLRootSignatureDecl *D) { + dumpName(D); +} + void TextNodeDumper::VisitHLSLOutArgExpr(const HLSLOutArgExpr *E) { OS << (E->isInOut() ? " inout" : " out"); } diff --git a/clang/lib/CodeGen/CGDecl.cpp b/clang/lib/CodeGen/CGDecl.cpp index fe8c3cb20add3..4a8f7f6a42ecb 100644 --- a/clang/lib/CodeGen/CGDecl.cpp +++ b/clang/lib/CodeGen/CGDecl.cpp @@ -106,6 +106,7 @@ void CodeGenFunction::EmitDecl(const Decl &D, bool EvaluateConditionDecl) { case Decl::Binding: case Decl::UnresolvedUsingIfExists: case Decl::HLSLBuffer: + case Decl::HLSLRootSignature: llvm_unreachable("Declaration should not be in declstmts!"); case Decl::Record: // struct/union/class X; case Decl::CXXRecord: // struct/union/class X; [C++] diff --git a/clang/lib/Parse/ParseDeclCXX.cpp b/clang/lib/Parse/ParseDeclCXX.cpp index 2aa7a5b1a0cb1..a4abdca2b7a3f 100644 --- a/clang/lib/Parse/ParseDeclCXX.cpp +++ b/clang/lib/Parse/ParseDeclCXX.cpp @@ -21,10 +21,12 @@ #include "clang/Basic/TargetInfo.h" #include "clang/Basic/TokenKinds.h" #include "clang/Lex/LiteralSupport.h" +#include "clang/Parse/ParseHLSLRootSignature.h" #include "clang/Parse/Parser.h" #include "clang/Parse/RAIIObjectsForParser.h" #include "clang/Sema/DeclSpec.h" #include "clang/Sema/EnterExpressionEvaluationContext.h" +#include "clang/Sema/Lookup.h" #include "clang/Sema/ParsedTemplate.h" #include "clang/Sema/Scope.h" #include "clang/Sema/SemaCodeCompletion.h" @@ -5311,6 +5313,92 @@ void Parser::ParseMicrosoftUuidAttributeArgs(ParsedAttributes &Attrs) { } } +void Parser::ParseMicrosoftRootSignatureAttributeArgs(ParsedAttributes &Attrs) { + assert(Tok.is(tok::identifier) && "Not a Microsoft attribute list"); + IdentifierInfo *RootSignatureIdent = Tok.getIdentifierInfo(); + assert(RootSignatureIdent->getName() == "RootSignature" && + "Not a Microsoft attribute list"); + + SourceLocation RootSignatureLoc = Tok.getLocation(); + ConsumeToken(); + + // Ignore the left paren location for now. + BalancedDelimiterTracker T(*this, tok::l_paren); + if (T.consumeOpen()) { + Diag(Tok, diag::err_expected) << tok::l_paren; + return; + } + + if (!isTokenStringLiteral()) { + Diag(Tok, diag::err_expected_string_literal) + << /*in attributes...*/ 4 << RootSignatureIdent->getName(); + SkipUntil(tok::r_square, StopAtSemi | StopBeforeMatch); + if (Tok.is(tok::r_paren)) + T.consumeClose(); + return; + } + + ExprResult StringResult = ParseUnevaluatedStringLiteralExpression(); + if (StringResult.isInvalid()) { + SkipUntil(tok::r_square, StopAtSemi | StopBeforeMatch); + if (Tok.is(tok::r_paren)) + T.consumeClose(); + return; + } + + ArgsVector Args; + if (auto Lit = dyn_cast<StringLiteral>(StringResult.get())) { + // Construct our identifier + StringRef Signature = Lit->getString(); + auto Hash = llvm::hash_value(Signature); + std::string IdStr = "__hlsl_rootsig_decl_" + std::to_string(Hash); + IdentifierInfo *DeclIdent = &(Actions.getASTContext().Idents.get(IdStr)); + + LookupResult R(Actions, DeclIdent, SourceLocation(), + Sema::LookupOrdinaryName); + // Check if we have already found a decl of the same name, if we haven't + // then parse the root signature string and construct the in-memory elements + if (!Actions.LookupQualifiedName(R, Actions.CurContext)) { + // Invoke the root signature parser to construct the in-memory constructs + hlsl::RootSignatureLexer Lexer(Signature, RootSignatureLoc); + SmallVector<llvm::hlsl::rootsig::RootElement> Elements; + hlsl::RootSignatureParser Parser(Elements, Lexer, PP); + if (Parser.parse()) { + SkipUntil(tok::r_square, StopAtSemi | StopBeforeMatch); + if (Tok.is(tok::r_paren)) + T.consumeClose(); + return; + } + + // Allocate the root elements onto ASTContext + unsigned N = Elements.size(); + auto RootElements = MutableArrayRef<llvm::hlsl::rootsig::RootElement>( + ::new (Actions.getASTContext()) llvm::hlsl::rootsig::RootElement[N], + N); + for (unsigned I = 0; I < N; ++I) + RootElements[I] = Elements[I]; + + // Create the Root Signature + auto *SignatureDecl = HLSLRootSignatureDecl::Create( + Actions.getASTContext(), /*FIXME?*/ Actions.CurContext, + RootSignatureLoc, DeclIdent, RootElements); + SignatureDecl->setImplicit(); + Actions.PushOnScopeChains(SignatureDecl, getCurScope()); + } + + // Create the arg for the ParsedAttr + IdentifierLoc *ILoc = ::new (Actions.getASTContext()) + IdentifierLoc(RootSignatureLoc, DeclIdent); + Args.push_back(ILoc); + } + + if (!T.consumeClose()) + Attrs.addNew(RootSignatureIdent, + SourceRange(RootSignatureLoc, T.getCloseLocation()), nullptr, + SourceLocation(), Args.data(), Args.size(), + ParsedAttr::Form::Microsoft()); +} + /// ParseMicrosoftAttributes - Parse Microsoft attributes [Attr] /// /// [MS] ms-attribute: @@ -5345,6 +5433,8 @@ void Parser::ParseMicrosoftAttributes(ParsedAttributes &Attrs) { break; if (Tok.getIdentifierInfo()->getName() == "uuid") ParseMicrosoftUuidAttributeArgs(Attrs); + else if (Tok.getIdentifierInfo()->getName() == "RootSignature") + ParseMicrosoftRootSignatureAttributeArgs(Attrs); else { IdentifierInfo *II = Tok.getIdentifierInfo(); SourceLocation NameLoc = Tok.getLocation(); diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp index b8801233942e1..377595639bef1 100644 --- a/clang/lib/Sema/SemaDeclAttr.cpp +++ b/clang/lib/Sema/SemaDeclAttr.cpp @@ -7481,6 +7481,9 @@ ProcessDeclAttribute(Sema &S, Scope *scope, Decl *D, const ParsedAttr &AL, break; // HLSL attributes: + case ParsedAttr::AT_RootSignature: + S.HLSL().handleRootSignatureAttr(D, AL); + break; case ParsedAttr::AT_HLSLNumThreads: S.HLSL().handleNumThreadsAttr(D, AL); break; diff --git a/clang/lib/Sema/SemaHLSL.cpp b/clang/lib/Sema/SemaHLSL.cpp index 08bd22a262788..24030a880048d 100644 --- a/clang/lib/Sema/SemaHLSL.cpp +++ b/clang/lib/Sema/SemaHLSL.cpp @@ -29,6 +29,7 @@ #include "clang/Basic/Specifiers.h" #include "clang/Basic/TargetInfo.h" #include "clang/Sema/Initialization.h" +#include "clang/Sema/Lookup.h" #include "clang/Sema/ParsedAttr.h" #include "clang/Sema/Sema.h" #include "clang/Sema/Template.h" @@ -950,6 +951,23 @@ void SemaHLSL::emitLogicalOperatorFixIt(Expr *LHS, Expr *RHS, << NewFnName << FixItHint::CreateReplacement(FullRange, OS.str()); } +void SemaHLSL::handleRootSignatureAttr(Decl *D, const ParsedAttr &AL) { + if (AL.getNumArgs() != 1) { + Diag(AL.getLoc(), diag::err_attribute_wrong_number_arguments) << AL << 1; + return; + } + + IdentifierInfo *Ident = AL.getArgAsIdent(0)->getIdentifierInfo(); + LookupResult R(SemaRef, Ident, SourceLocation(), Sema::LookupOrdinaryName); + if (SemaRef.LookupQualifiedName(R, D->getDeclContext())) + if (auto *SignatureDecl = + dyn_cast<HLSLRootSignatureDecl>(R.getFoundDecl())) { + // Perform validation of constructs here + D->addAttr(::new (getASTContext()) + RootSignatureAttr(getASTContext(), AL, Ident)); + } +} + void SemaHLSL::handleNumThreadsAttr(Decl *D, const ParsedAttr &AL) { llvm::VersionTuple SMVersion = getASTContext().getTargetInfo().getTriple().getOSVersion(); diff --git a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp index 08b3a423d1526..01065f22b34a8 100644 --- a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp +++ b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp @@ -999,6 +999,11 @@ Decl *TemplateDeclInstantiator::VisitHLSLBufferDecl(HLSLBufferDecl *Decl) { llvm_unreachable("HLSL buffer declarations cannot be instantiated"); } +Decl *TemplateDeclInstantiator::VisitHLSLRootSignatureDecl( + HLSLRootSignatureDecl *Decl) { + llvm_unreachable("HLSL root signature declarations cannot be instantiated"); +} + Decl * TemplateDeclInstantiator::VisitPragmaCommentDecl(PragmaCommentDecl *D) { llvm_unreachable("pragma comment cannot be instantiated"); diff --git a/clang/lib/Serialization/ASTCommon.cpp b/clang/lib/Serialization/ASTCommon.cpp index ad277f19711ff..76eb8697b2afd 100644 --- a/clang/lib/Serialization/ASTCommon.cpp +++ b/clang/lib/Serialization/ASTCommon.cpp @@ -458,6 +458,7 @@ bool serialization::isRedeclarableDeclKind(unsigned Kind) { case Decl::RequiresExprBody: case Decl::UnresolvedUsingIfExists: case Decl::HLSLBuffer: + case Decl::HLSLRootSignature: case Decl::OpenACCDeclare: case Decl::OpenACCRoutine: return false; diff --git a/clang/test/AST/HLSL/RootSignatures-AST.hlsl b/clang/test/AST/HLSL/RootSignatures-AST.hlsl new file mode 100644 index 0000000000000..e6ed27b62588a --- /dev/null +++ b/clang/test/AST/HLSL/RootSignatures-AST.hlsl @@ -0,0 +1,58 @@ +// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.0-library -ast-dump \ +// RUN: -disable-llvm-passes -o - %s | FileCheck %s + +// This test ensures that the sample root signature is parsed without error and +// the Attr AST Node is created succesfully. If an invalid root signature was +// passed in then we would exit out of Sema before the Attr is created. + +#define SampleRS \ + "DescriptorTable( " \ + " CBV(b1), " \ + " SRV(t1, numDescriptors = 8, " \ + " flags = DESCRIPTORS_VOLATILE), " \ + " UAV(u1, numDescriptors = 0, " \ + " flags = DESCRIPTORS_VOLATILE) " \ + "), " \ + "DescriptorTable(Sampler(s0, numDescriptors = 4, space = 1))" + +// CHECK: -HLSLRootSignatureDecl 0x{{.*}} {{.*}} implicit [[SAMPLE_RS_DECL:__hlsl_rootsig_decl_.*]] + +// CHECK: -RootSignatureAttr 0x{{.*}} {{.*}} [[SAMPLE_RS_DECL]] +[RootSignature(SampleRS)] +void rs_main() {} + +// Ensure that if multiple root signatures are specified at different entry +// points that we point to the correct root signature + +// CHECK: -RootSignatureAttr 0x{{.*}} {{.*}} [[SAMPLE_RS_DECL]] +[RootSignature(SampleRS)] +void same_rs_main() {} + +// Define the same root signature to ensure that the entry point will still +// link to the same root signature declaration + +#define SampleSameRS \ + "DescriptorTable( " \ + " CBV(b1), " \ + " SRV(t1, numDescriptors = 8, " \ + " flags = DESCRIPTORS_VOLATILE), " \ + " UAV(u1, numDescriptors = 0, " \ + " flags = DESCRIPTORS_VOLATILE) " \ + "), " \ + "DescriptorTable(Sampler(s0, numDescriptors = 4, space = 1))" + +// CHECK: -RootSignatureAttr 0x{{.*}} {{.*}} [[SAMPLE_RS_DECL]] +[RootSignature(SampleSameRS)] +void same_rs_string_main() {} + +#define SampleDifferentRS \ + "DescriptorTable(Sampler(s0, numDescriptors = 4, space = 1))" + +// Ensure that when we define a different type root signature that it creates +// a seperate decl and identifier to reference + +// CHECK: -HLSLRootSignatureDecl 0x{{.*}} {{.*}} implicit [[DIFF_RS_DECL:__hlsl_rootsig_decl_.*]] + +// CHECK: -RootSignatureAttr 0x{{.*}} {{.*}} [[DIFF_RS_DECL]] +[RootSignature(SampleDifferentRS)] +void different_rs_string_main() {} diff --git a/clang/test/SemaHLSL/RootSignature-err.hlsl b/clang/test/SemaHLSL/RootSignature-err.hlsl new file mode 100644 index 0000000000000..6427c78b38455 --- /dev/null +++ b/clang/test/SemaHLSL/RootSignature-err.hlsl @@ -0,0 +1,11 @@ +// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.3-library -x hlsl -o - %s -verify + +// Attr test + +[RootSignature()] // expected-error {{expected string literal as argument of 'RootSignature' attribute}} +void bad_root_signature_0() {} + +// expected-error@+2 {{expected ')'}} +// expected-note@+1 {{to match this '('}} +[RootSignature("", "")] +void bad_root_signature_1() {} diff --git a/clang/tools/libclang/CIndex.cpp b/clang/tools/libclang/CIndex.cpp index fa5df3b5a06e6..43ac6880ca1da 100644 --- a/clang/tools/libclang/CIndex.cpp +++ b/clang/tools/libclang/CIndex.cpp @@ -7230,6 +7230,7 @@ CXCursor clang_getCursorDefinition(CXCursor C) { case Decl::MSProperty: case Decl::MSGuid: case Decl::HLSLBuffer: + case Decl::HLSLRootSignature: case Decl::UnnamedGlobalConstant: case Decl::TemplateParamObject: case Decl::IndirectField: >From 17694ecb15ffa412a7143253c27dc04757850e4d Mon Sep 17 00:00:00 2001 From: Finn Plummer <canadienf...@gmail.com> Date: Mon, 28 Apr 2025 19:08:12 +0000 Subject: [PATCH 02/16] clang format --- clang/lib/AST/TextNodeDumper.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/clang/lib/AST/TextNodeDumper.cpp b/clang/lib/AST/TextNodeDumper.cpp index 53364719369de..36d5027936aba 100644 --- a/clang/lib/AST/TextNodeDumper.cpp +++ b/clang/lib/AST/TextNodeDumper.cpp @@ -3037,7 +3037,8 @@ void TextNodeDumper::VisitHLSLBufferDecl(const HLSLBufferDecl *D) { dumpName(D); } -void TextNodeDumper::VisitHLSLRootSignatureDecl(const HLSLRootSignatureDecl *D) { +void TextNodeDumper::VisitHLSLRootSignatureDecl( + const HLSLRootSignatureDecl *D) { dumpName(D); } >From 0990882b3792d948e2bb2edf7b42d9eefedb9443 Mon Sep 17 00:00:00 2001 From: Finn Plummer <canadienf...@gmail.com> Date: Wed, 30 Apr 2025 18:29:57 +0000 Subject: [PATCH 03/16] review: improve assert messages --- clang/lib/Parse/ParseDeclCXX.cpp | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/clang/lib/Parse/ParseDeclCXX.cpp b/clang/lib/Parse/ParseDeclCXX.cpp index a4abdca2b7a3f..f72c671f3ff50 100644 --- a/clang/lib/Parse/ParseDeclCXX.cpp +++ b/clang/lib/Parse/ParseDeclCXX.cpp @@ -5314,10 +5314,11 @@ void Parser::ParseMicrosoftUuidAttributeArgs(ParsedAttributes &Attrs) { } void Parser::ParseMicrosoftRootSignatureAttributeArgs(ParsedAttributes &Attrs) { - assert(Tok.is(tok::identifier) && "Not a Microsoft attribute list"); + assert(Tok.is(tok::identifier) && + "Expected an identifier to denote which MS attribute to consider"); IdentifierInfo *RootSignatureIdent = Tok.getIdentifierInfo(); assert(RootSignatureIdent->getName() == "RootSignature" && - "Not a Microsoft attribute list"); + "Expected RootSignature identifier for root signature attribute"); SourceLocation RootSignatureLoc = Tok.getLocation(); ConsumeToken(); >From 8c693f023722673c7ef5fabe2d9067de210f739b Mon Sep 17 00:00:00 2001 From: Finn Plummer <canadienf...@gmail.com> Date: Wed, 30 Apr 2025 18:33:17 +0000 Subject: [PATCH 04/16] review: remove FIXME - this is correct as we do want to add the newly constructed Declaration into the current decl context, such that we can retrieve it later when we look it up from the function declaration in the same context --- clang/lib/Parse/ParseDeclCXX.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clang/lib/Parse/ParseDeclCXX.cpp b/clang/lib/Parse/ParseDeclCXX.cpp index f72c671f3ff50..69bac6d008ae7 100644 --- a/clang/lib/Parse/ParseDeclCXX.cpp +++ b/clang/lib/Parse/ParseDeclCXX.cpp @@ -5381,7 +5381,7 @@ void Parser::ParseMicrosoftRootSignatureAttributeArgs(ParsedAttributes &Attrs) { // Create the Root Signature auto *SignatureDecl = HLSLRootSignatureDecl::Create( - Actions.getASTContext(), /*FIXME?*/ Actions.CurContext, + Actions.getASTContext(), /*DeclContext=*/Actions.CurContext, RootSignatureLoc, DeclIdent, RootElements); SignatureDecl->setImplicit(); Actions.PushOnScopeChains(SignatureDecl, getCurScope()); >From 5b05ef67f86a6446c2ad8daf815998775085566b Mon Sep 17 00:00:00 2001 From: Finn Plummer <canadienf...@gmail.com> Date: Wed, 30 Apr 2025 18:48:12 +0000 Subject: [PATCH 05/16] review: unify error reporting of having an invalid string literal argument --- clang/lib/Parse/ParseDeclCXX.cpp | 107 ++++++++++++++++--------------- 1 file changed, 56 insertions(+), 51 deletions(-) diff --git a/clang/lib/Parse/ParseDeclCXX.cpp b/clang/lib/Parse/ParseDeclCXX.cpp index 69bac6d008ae7..d558347de13b5 100644 --- a/clang/lib/Parse/ParseDeclCXX.cpp +++ b/clang/lib/Parse/ParseDeclCXX.cpp @@ -5330,7 +5330,22 @@ void Parser::ParseMicrosoftRootSignatureAttributeArgs(ParsedAttributes &Attrs) { return; } - if (!isTokenStringLiteral()) { + auto ProcessStringLiteral = [this]() -> std::optional<StringLiteral *> { + if (!isTokenStringLiteral()) + return std::nullopt; + + ExprResult StringResult = ParseUnevaluatedStringLiteralExpression(); + if (StringResult.isInvalid()) + return std::nullopt; + + if (auto Lit = dyn_cast<StringLiteral>(StringResult.get())) + return Lit; + + return std::nullopt; + }; + + auto StrLiteral = ProcessStringLiteral(); + if (!StrLiteral.has_value()) { Diag(Tok, diag::err_expected_string_literal) << /*in attributes...*/ 4 << RootSignatureIdent->getName(); SkipUntil(tok::r_square, StopAtSemi | StopBeforeMatch); @@ -5338,60 +5353,50 @@ void Parser::ParseMicrosoftRootSignatureAttributeArgs(ParsedAttributes &Attrs) { T.consumeClose(); return; } + + // Construct our identifier + StringRef Signature = StrLiteral.value()->getString(); + auto Hash = llvm::hash_value(Signature); + std::string IdStr = "__hlsl_rootsig_decl_" + std::to_string(Hash); + IdentifierInfo *DeclIdent = &(Actions.getASTContext().Idents.get(IdStr)); + + LookupResult R(Actions, DeclIdent, SourceLocation(), + Sema::LookupOrdinaryName); + // Check if we have already found a decl of the same name, if we haven't + // then parse the root signature string and construct the in-memory elements + if (!Actions.LookupQualifiedName(R, Actions.CurContext)) { + // Invoke the root signature parser to construct the in-memory constructs + hlsl::RootSignatureLexer Lexer(Signature, RootSignatureLoc); + SmallVector<llvm::hlsl::rootsig::RootElement> Elements; + hlsl::RootSignatureParser Parser(Elements, Lexer, PP); + if (Parser.parse()) { + SkipUntil(tok::r_square, StopAtSemi | StopBeforeMatch); + if (Tok.is(tok::r_paren)) + T.consumeClose(); + return; + } - ExprResult StringResult = ParseUnevaluatedStringLiteralExpression(); - if (StringResult.isInvalid()) { - SkipUntil(tok::r_square, StopAtSemi | StopBeforeMatch); - if (Tok.is(tok::r_paren)) - T.consumeClose(); - return; - } + // Allocate the root elements onto ASTContext + unsigned N = Elements.size(); + auto RootElements = MutableArrayRef<llvm::hlsl::rootsig::RootElement>( + ::new (Actions.getASTContext()) llvm::hlsl::rootsig::RootElement[N], + N); + for (unsigned I = 0; I < N; ++I) + RootElements[I] = Elements[I]; - ArgsVector Args; - if (auto Lit = dyn_cast<StringLiteral>(StringResult.get())) { - // Construct our identifier - StringRef Signature = Lit->getString(); - auto Hash = llvm::hash_value(Signature); - std::string IdStr = "__hlsl_rootsig_decl_" + std::to_string(Hash); - IdentifierInfo *DeclIdent = &(Actions.getASTContext().Idents.get(IdStr)); - - LookupResult R(Actions, DeclIdent, SourceLocation(), - Sema::LookupOrdinaryName); - // Check if we have already found a decl of the same name, if we haven't - // then parse the root signature string and construct the in-memory elements - if (!Actions.LookupQualifiedName(R, Actions.CurContext)) { - // Invoke the root signature parser to construct the in-memory constructs - hlsl::RootSignatureLexer Lexer(Signature, RootSignatureLoc); - SmallVector<llvm::hlsl::rootsig::RootElement> Elements; - hlsl::RootSignatureParser Parser(Elements, Lexer, PP); - if (Parser.parse()) { - SkipUntil(tok::r_square, StopAtSemi | StopBeforeMatch); - if (Tok.is(tok::r_paren)) - T.consumeClose(); - return; - } + // Create the Root Signature + auto *SignatureDecl = HLSLRootSignatureDecl::Create( + Actions.getASTContext(), /*DeclContext=*/Actions.CurContext, + RootSignatureLoc, DeclIdent, RootElements); + SignatureDecl->setImplicit(); + Actions.PushOnScopeChains(SignatureDecl, getCurScope()); + } - // Allocate the root elements onto ASTContext - unsigned N = Elements.size(); - auto RootElements = MutableArrayRef<llvm::hlsl::rootsig::RootElement>( - ::new (Actions.getASTContext()) llvm::hlsl::rootsig::RootElement[N], - N); - for (unsigned I = 0; I < N; ++I) - RootElements[I] = Elements[I]; - - // Create the Root Signature - auto *SignatureDecl = HLSLRootSignatureDecl::Create( - Actions.getASTContext(), /*DeclContext=*/Actions.CurContext, - RootSignatureLoc, DeclIdent, RootElements); - SignatureDecl->setImplicit(); - Actions.PushOnScopeChains(SignatureDecl, getCurScope()); - } + // Create the arg for the ParsedAttr + IdentifierLoc *ILoc = ::new (Actions.getASTContext()) + IdentifierLoc(RootSignatureLoc, DeclIdent); - // Create the arg for the ParsedAttr - IdentifierLoc *ILoc = ::new (Actions.getASTContext()) - IdentifierLoc(RootSignatureLoc, DeclIdent); - Args.push_back(ILoc); - } + ArgsVector Args = { ILoc }; if (!T.consumeClose()) Attrs.addNew(RootSignatureIdent, >From 141c7323b80d733d3e539125147107f44aed55fc Mon Sep 17 00:00:00 2001 From: Finn Plummer <canadienf...@gmail.com> Date: Wed, 30 Apr 2025 18:48:41 +0000 Subject: [PATCH 06/16] clang-format --- clang/lib/Parse/ParseDeclCXX.cpp | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/clang/lib/Parse/ParseDeclCXX.cpp b/clang/lib/Parse/ParseDeclCXX.cpp index d558347de13b5..8834eacd1dfff 100644 --- a/clang/lib/Parse/ParseDeclCXX.cpp +++ b/clang/lib/Parse/ParseDeclCXX.cpp @@ -5337,7 +5337,7 @@ void Parser::ParseMicrosoftRootSignatureAttributeArgs(ParsedAttributes &Attrs) { ExprResult StringResult = ParseUnevaluatedStringLiteralExpression(); if (StringResult.isInvalid()) return std::nullopt; - + if (auto Lit = dyn_cast<StringLiteral>(StringResult.get())) return Lit; @@ -5353,7 +5353,7 @@ void Parser::ParseMicrosoftRootSignatureAttributeArgs(ParsedAttributes &Attrs) { T.consumeClose(); return; } - + // Construct our identifier StringRef Signature = StrLiteral.value()->getString(); auto Hash = llvm::hash_value(Signature); @@ -5379,8 +5379,7 @@ void Parser::ParseMicrosoftRootSignatureAttributeArgs(ParsedAttributes &Attrs) { // Allocate the root elements onto ASTContext unsigned N = Elements.size(); auto RootElements = MutableArrayRef<llvm::hlsl::rootsig::RootElement>( - ::new (Actions.getASTContext()) llvm::hlsl::rootsig::RootElement[N], - N); + ::new (Actions.getASTContext()) llvm::hlsl::rootsig::RootElement[N], N); for (unsigned I = 0; I < N; ++I) RootElements[I] = Elements[I]; @@ -5396,7 +5395,7 @@ void Parser::ParseMicrosoftRootSignatureAttributeArgs(ParsedAttributes &Attrs) { IdentifierLoc *ILoc = ::new (Actions.getASTContext()) IdentifierLoc(RootSignatureLoc, DeclIdent); - ArgsVector Args = { ILoc }; + ArgsVector Args = {ILoc}; if (!T.consumeClose()) Attrs.addNew(RootSignatureIdent, >From 6e842d7b5f399b1bf0a5f999a99d9785459550a2 Mon Sep 17 00:00:00 2001 From: Finn Plummer <canadienf...@gmail.com> Date: Wed, 30 Apr 2025 19:10:19 +0000 Subject: [PATCH 07/16] review: use TrailingObjects to store elements instead of on the context --- clang/include/clang/AST/Decl.h | 27 ++++++++++++++++++++------- clang/lib/AST/Decl.cpp | 25 ++++++++++++++++--------- 2 files changed, 36 insertions(+), 16 deletions(-) diff --git a/clang/include/clang/AST/Decl.h b/clang/include/clang/AST/Decl.h index 8e45c19061b1d..f1013c57e008f 100644 --- a/clang/include/clang/AST/Decl.h +++ b/clang/include/clang/AST/Decl.h @@ -5179,22 +5179,35 @@ class HLSLBufferDecl final : public NamedDecl, public DeclContext { friend class ASTDeclWriter; }; -class HLSLRootSignatureDecl final : public NamedDecl { - ArrayRef<llvm::hlsl::rootsig::RootElement> RootElements; +class HLSLRootSignatureDecl final + : public NamedDecl, + private llvm::TrailingObjects<HLSLRootSignatureDecl, + llvm::hlsl::rootsig::RootElement> { + friend TrailingObjects; + + unsigned NumElems; - HLSLRootSignatureDecl( - DeclContext *DC, SourceLocation Loc, IdentifierInfo *ID, - ArrayRef<llvm::hlsl::rootsig::RootElement> RootElements); + llvm::hlsl::rootsig::RootElement *getElems() { + return getTrailingObjects<llvm::hlsl::rootsig::RootElement>(); + } + + const llvm::hlsl::rootsig::RootElement *getElems() const { + return getTrailingObjects<llvm::hlsl::rootsig::RootElement>(); + } + + HLSLRootSignatureDecl(DeclContext *DC, SourceLocation Loc, IdentifierInfo *ID, + unsigned NumElems); public: static HLSLRootSignatureDecl * Create(ASTContext &C, DeclContext *DC, SourceLocation Loc, IdentifierInfo *ID, ArrayRef<llvm::hlsl::rootsig::RootElement> RootElements); + static HLSLRootSignatureDecl *CreateDeserialized(ASTContext &C, GlobalDeclID ID); - ArrayRef<llvm::hlsl::rootsig::RootElement> &getRootElements() { - return RootElements; + ArrayRef<llvm::hlsl::rootsig::RootElement> getRootElements() const { + return {getElems(), NumElems}; } // Implement isa/cast/dyncast/etc. diff --git a/clang/lib/AST/Decl.cpp b/clang/lib/AST/Decl.cpp index 8bf881a1b5f43..061fedb403ddd 100644 --- a/clang/lib/AST/Decl.cpp +++ b/clang/lib/AST/Decl.cpp @@ -5851,24 +5851,31 @@ bool HLSLBufferDecl::buffer_decls_empty() { // HLSLRootSignatureDecl Implementation //===----------------------------------------------------------------------===// -HLSLRootSignatureDecl::HLSLRootSignatureDecl( - DeclContext *DC, SourceLocation Loc, IdentifierInfo *ID, - ArrayRef<llvm::hlsl::rootsig::RootElement> RootElements) +HLSLRootSignatureDecl::HLSLRootSignatureDecl(DeclContext *DC, + SourceLocation Loc, + IdentifierInfo *ID, + unsigned NumElems) : NamedDecl(Decl::Kind::HLSLRootSignature, DC, Loc, DeclarationName(ID)), - RootElements(RootElements) {} + NumElems(NumElems) {} HLSLRootSignatureDecl *HLSLRootSignatureDecl::Create( ASTContext &C, DeclContext *DC, SourceLocation Loc, IdentifierInfo *ID, ArrayRef<llvm::hlsl::rootsig::RootElement> RootElements) { - HLSLRootSignatureDecl *Result = - new (C, DC) HLSLRootSignatureDecl(DC, Loc, ID, RootElements); - return Result; + HLSLRootSignatureDecl *RSDecl = + new (C, DC, + additionalSizeToAlloc<llvm::hlsl::rootsig::RootElement>( + RootElements.size())) + HLSLRootSignatureDecl(DC, Loc, ID, RootElements.size()); + auto *StoredElems = RSDecl->getElems(); + std::uninitialized_copy(RootElements.begin(), RootElements.end(), + StoredElems); + return RSDecl; } HLSLRootSignatureDecl * HLSLRootSignatureDecl::CreateDeserialized(ASTContext &C, GlobalDeclID ID) { - HLSLRootSignatureDecl *Result = - new (C, ID) HLSLRootSignatureDecl(nullptr, SourceLocation(), nullptr, {}); + HLSLRootSignatureDecl *Result = new (C, ID) + HLSLRootSignatureDecl(nullptr, SourceLocation(), nullptr, /*NumElems=*/0); return Result; } >From a0fd705fb6c8c220e3d638699862d02867f0b269 Mon Sep 17 00:00:00 2001 From: Finn Plummer <canadienf...@gmail.com> Date: Wed, 30 Apr 2025 21:10:08 +0000 Subject: [PATCH 08/16] review: update duplicate attrs warning/error --- clang/lib/Sema/SemaHLSL.cpp | 10 ++++++++++ clang/test/SemaHLSL/RootSignature-err.hlsl | 6 ++++++ 2 files changed, 16 insertions(+) diff --git a/clang/lib/Sema/SemaHLSL.cpp b/clang/lib/Sema/SemaHLSL.cpp index 24030a880048d..e295114d7617b 100644 --- a/clang/lib/Sema/SemaHLSL.cpp +++ b/clang/lib/Sema/SemaHLSL.cpp @@ -958,6 +958,16 @@ void SemaHLSL::handleRootSignatureAttr(Decl *D, const ParsedAttr &AL) { } IdentifierInfo *Ident = AL.getArgAsIdent(0)->getIdentifierInfo(); + if (auto *RS = D->getAttr<RootSignatureAttr>()) { + if (RS->getSignature() != Ident) { + Diag(AL.getLoc(), diag::err_disallowed_duplicate_attribute) << RS; + return; + } + + Diag(AL.getLoc(), diag::warn_duplicate_attribute_exact) << RS; + return; + } + LookupResult R(SemaRef, Ident, SourceLocation(), Sema::LookupOrdinaryName); if (SemaRef.LookupQualifiedName(R, D->getDeclContext())) if (auto *SignatureDecl = diff --git a/clang/test/SemaHLSL/RootSignature-err.hlsl b/clang/test/SemaHLSL/RootSignature-err.hlsl index 6427c78b38455..21af0a6552bbc 100644 --- a/clang/test/SemaHLSL/RootSignature-err.hlsl +++ b/clang/test/SemaHLSL/RootSignature-err.hlsl @@ -9,3 +9,9 @@ void bad_root_signature_0() {} // expected-note@+1 {{to match this '('}} [RootSignature("", "")] void bad_root_signature_1() {} + +[RootSignature(""), RootSignature("DescriptorTable()")] // expected-error {{attribute 'RootSignature' cannot appear more than once on a declaration}} +void bad_root_signature_2() {} + +[RootSignature(""), RootSignature("")] // expected-warning {{attribute 'RootSignature' is already applied}} +void bad_root_signature_3() {} >From 80609f44b4a74fe691aa86cf8a976af19305a97c Mon Sep 17 00:00:00 2001 From: Finn Plummer <canadienf...@gmail.com> Date: Wed, 30 Apr 2025 22:39:35 +0000 Subject: [PATCH 09/16] self-review: correct retrieved start location - increment the location by one to better point past the initial " character of a string literal - add test that demonstrates an HLSLRootSignature parsing error being propogated up and it correctly continues parsing after the closed paren enclosing the string literal --- clang/lib/Parse/ParseDeclCXX.cpp | 4 +++- clang/test/SemaHLSL/RootSignature-err.hlsl | 3 +++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/clang/lib/Parse/ParseDeclCXX.cpp b/clang/lib/Parse/ParseDeclCXX.cpp index 8834eacd1dfff..2434e375e6e72 100644 --- a/clang/lib/Parse/ParseDeclCXX.cpp +++ b/clang/lib/Parse/ParseDeclCXX.cpp @@ -5365,8 +5365,10 @@ void Parser::ParseMicrosoftRootSignatureAttributeArgs(ParsedAttributes &Attrs) { // Check if we have already found a decl of the same name, if we haven't // then parse the root signature string and construct the in-memory elements if (!Actions.LookupQualifiedName(R, Actions.CurContext)) { + SourceLocation SignatureLoc = + StrLiteral.value()->getExprLoc().getLocWithOffset(1); // offset 1 for '"' // Invoke the root signature parser to construct the in-memory constructs - hlsl::RootSignatureLexer Lexer(Signature, RootSignatureLoc); + hlsl::RootSignatureLexer Lexer(Signature, SignatureLoc); SmallVector<llvm::hlsl::rootsig::RootElement> Elements; hlsl::RootSignatureParser Parser(Elements, Lexer, PP); if (Parser.parse()) { diff --git a/clang/test/SemaHLSL/RootSignature-err.hlsl b/clang/test/SemaHLSL/RootSignature-err.hlsl index 21af0a6552bbc..f544247f4db2a 100644 --- a/clang/test/SemaHLSL/RootSignature-err.hlsl +++ b/clang/test/SemaHLSL/RootSignature-err.hlsl @@ -15,3 +15,6 @@ void bad_root_signature_2() {} [RootSignature(""), RootSignature("")] // expected-warning {{attribute 'RootSignature' is already applied}} void bad_root_signature_3() {} + +[RootSignature("DescriptorTable(), invalid")] // expected-error {{expected end of stream to denote end of parameters, or, another valid parameter of RootSignature}} +void bad_root_signature_4() {} >From d42f2703b8d7c0371e65fa7d87766aad799eca93 Mon Sep 17 00:00:00 2001 From: Finn Plummer <canadienf...@gmail.com> Date: Wed, 30 Apr 2025 22:40:26 +0000 Subject: [PATCH 10/16] self-review: update skip logic to account for context --- clang/lib/Parse/ParseDeclCXX.cpp | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/clang/lib/Parse/ParseDeclCXX.cpp b/clang/lib/Parse/ParseDeclCXX.cpp index 2434e375e6e72..f67dadfe45fc3 100644 --- a/clang/lib/Parse/ParseDeclCXX.cpp +++ b/clang/lib/Parse/ParseDeclCXX.cpp @@ -5348,9 +5348,8 @@ void Parser::ParseMicrosoftRootSignatureAttributeArgs(ParsedAttributes &Attrs) { if (!StrLiteral.has_value()) { Diag(Tok, diag::err_expected_string_literal) << /*in attributes...*/ 4 << RootSignatureIdent->getName(); - SkipUntil(tok::r_square, StopAtSemi | StopBeforeMatch); - if (Tok.is(tok::r_paren)) - T.consumeClose(); + SkipUntil(tok::r_paren, StopAtSemi | StopBeforeMatch); + T.consumeClose(); return; } @@ -5372,9 +5371,7 @@ void Parser::ParseMicrosoftRootSignatureAttributeArgs(ParsedAttributes &Attrs) { SmallVector<llvm::hlsl::rootsig::RootElement> Elements; hlsl::RootSignatureParser Parser(Elements, Lexer, PP); if (Parser.parse()) { - SkipUntil(tok::r_square, StopAtSemi | StopBeforeMatch); - if (Tok.is(tok::r_paren)) - T.consumeClose(); + T.consumeClose(); return; } >From d80980669f16dddc329ed51b5da86aab56ef8a65 Mon Sep 17 00:00:00 2001 From: Finn Plummer <canadienf...@gmail.com> Date: Thu, 1 May 2025 00:02:40 +0000 Subject: [PATCH 11/16] clang-format --- clang/lib/Parse/ParseDeclCXX.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/clang/lib/Parse/ParseDeclCXX.cpp b/clang/lib/Parse/ParseDeclCXX.cpp index f67dadfe45fc3..8f1c86ec3bb3e 100644 --- a/clang/lib/Parse/ParseDeclCXX.cpp +++ b/clang/lib/Parse/ParseDeclCXX.cpp @@ -5365,7 +5365,8 @@ void Parser::ParseMicrosoftRootSignatureAttributeArgs(ParsedAttributes &Attrs) { // then parse the root signature string and construct the in-memory elements if (!Actions.LookupQualifiedName(R, Actions.CurContext)) { SourceLocation SignatureLoc = - StrLiteral.value()->getExprLoc().getLocWithOffset(1); // offset 1 for '"' + StrLiteral.value()->getExprLoc().getLocWithOffset( + 1); // offset 1 for '"' // Invoke the root signature parser to construct the in-memory constructs hlsl::RootSignatureLexer Lexer(Signature, SignatureLoc); SmallVector<llvm::hlsl::rootsig::RootElement> Elements; >From 45b2d921b008170d70fb6c2af1ea5fa4760e7d3c Mon Sep 17 00:00:00 2001 From: Finn Plummer <canadienf...@gmail.com> Date: Fri, 2 May 2025 19:49:20 +0000 Subject: [PATCH 12/16] rebase: hook-up root elements to invoke there dump - update ast test to ensure elements are correctly allocated --- clang/lib/AST/CMakeLists.txt | 1 + clang/lib/AST/TextNodeDumper.cpp | 2 ++ clang/test/AST/HLSL/RootSignatures-AST.hlsl | 21 +++++++++++++-- .../llvm/Frontend/HLSL/HLSLRootSignature.h | 3 +++ llvm/lib/Frontend/HLSL/HLSLRootSignature.cpp | 27 +++++++++++++++++++ 5 files changed, 52 insertions(+), 2 deletions(-) diff --git a/clang/lib/AST/CMakeLists.txt b/clang/lib/AST/CMakeLists.txt index 26d4d04d46e95..b5cd14b915673 100644 --- a/clang/lib/AST/CMakeLists.txt +++ b/clang/lib/AST/CMakeLists.txt @@ -2,6 +2,7 @@ set(LLVM_LINK_COMPONENTS BinaryFormat Core FrontendOpenMP + FrontendHLSL Support TargetParser ) diff --git a/clang/lib/AST/TextNodeDumper.cpp b/clang/lib/AST/TextNodeDumper.cpp index 36d5027936aba..112e902dfb374 100644 --- a/clang/lib/AST/TextNodeDumper.cpp +++ b/clang/lib/AST/TextNodeDumper.cpp @@ -24,6 +24,7 @@ #include "clang/Basic/Specifiers.h" #include "clang/Basic/TypeTraits.h" #include "llvm/ADT/StringExtras.h" +#include "llvm/Frontend/HLSL/HLSLRootSignature.h" #include <algorithm> #include <utility> @@ -3040,6 +3041,7 @@ void TextNodeDumper::VisitHLSLBufferDecl(const HLSLBufferDecl *D) { void TextNodeDumper::VisitHLSLRootSignatureDecl( const HLSLRootSignatureDecl *D) { dumpName(D); + llvm::hlsl::rootsig::dumpRootElements(OS, D->getRootElements()); } void TextNodeDumper::VisitHLSLOutArgExpr(const HLSLOutArgExpr *E) { diff --git a/clang/test/AST/HLSL/RootSignatures-AST.hlsl b/clang/test/AST/HLSL/RootSignatures-AST.hlsl index e6ed27b62588a..c700174da764d 100644 --- a/clang/test/AST/HLSL/RootSignatures-AST.hlsl +++ b/clang/test/AST/HLSL/RootSignatures-AST.hlsl @@ -15,7 +15,19 @@ "), " \ "DescriptorTable(Sampler(s0, numDescriptors = 4, space = 1))" -// CHECK: -HLSLRootSignatureDecl 0x{{.*}} {{.*}} implicit [[SAMPLE_RS_DECL:__hlsl_rootsig_decl_.*]] +// CHECK: -HLSLRootSignatureDecl 0x{{.*}} {{.*}} implicit [[SAMPLE_RS_DECL:__hlsl_rootsig_decl_\d*]] +// CHECK-SAME: RootElements{ +// CHECK-SAME: CBV(b1, numDescriptors = 1, space = 0, +// CHECK-SAME: offset = DescriptorTableOffsetAppend, flags = DataStaticWhileSetAtExecute), +// CHECK-SAME: SRV(t1, numDescriptors = 8, space = 0, +// CHECK-SAME: offset = DescriptorTableOffsetAppend, flags = DescriptorsVolatile), +// CHECK-SAME: UAV(u1, numDescriptors = 0, space = 0, +// CHECK-SAME: offset = DescriptorTableOffsetAppend, flags = DescriptorsVolatile), +// CHECK-SAME: DescriptorTable(numClauses = 3, visibility = All), +// CHECK-SAME: Sampler(s0, numDescriptors = 4, space = 1, +// CHECK-SAME: offset = DescriptorTableOffsetAppend, flags = None), +// CHECK-SAME: DescriptorTable(numClauses = 1, visibility = All) +// CHECK-SAME: } // CHECK: -RootSignatureAttr 0x{{.*}} {{.*}} [[SAMPLE_RS_DECL]] [RootSignature(SampleRS)] @@ -51,7 +63,12 @@ void same_rs_string_main() {} // Ensure that when we define a different type root signature that it creates // a seperate decl and identifier to reference -// CHECK: -HLSLRootSignatureDecl 0x{{.*}} {{.*}} implicit [[DIFF_RS_DECL:__hlsl_rootsig_decl_.*]] +// CHECK: -HLSLRootSignatureDecl 0x{{.*}} {{.*}} implicit [[DIFF_RS_DECL:__hlsl_rootsig_decl_\d*]] +// CHECK-SAME: RootElements{ +// CHECK-SAME: Sampler(s0, numDescriptors = 4, space = 1, +// CHECK-SAME: offset = DescriptorTableOffsetAppend, flags = None), +// CHECK-SAME: DescriptorTable(numClauses = 1, visibility = All) +// CHECK-SAME: } // CHECK: -RootSignatureAttr 0x{{.*}} {{.*}} [[DIFF_RS_DECL]] [RootSignature(SampleDifferentRS)] diff --git a/llvm/include/llvm/Frontend/HLSL/HLSLRootSignature.h b/llvm/include/llvm/Frontend/HLSL/HLSLRootSignature.h index be0ec5d072ed0..37f3d9ad61d3e 100644 --- a/llvm/include/llvm/Frontend/HLSL/HLSLRootSignature.h +++ b/llvm/include/llvm/Frontend/HLSL/HLSLRootSignature.h @@ -14,6 +14,7 @@ #ifndef LLVM_FRONTEND_HLSL_HLSLROOTSIGNATURE_H #define LLVM_FRONTEND_HLSL_HLSLROOTSIGNATURE_H +#include "llvm/ADT/ArrayRef.h" #include "llvm/Support/DXILABI.h" #include "llvm/Support/raw_ostream.h" #include <variant> @@ -122,6 +123,8 @@ struct DescriptorTableClause { using RootElement = std::variant<RootFlags, RootConstants, DescriptorTable, DescriptorTableClause>; +void dumpRootElements(raw_ostream &OS, ArrayRef<RootElement> Elements); + } // namespace rootsig } // namespace hlsl } // namespace llvm diff --git a/llvm/lib/Frontend/HLSL/HLSLRootSignature.cpp b/llvm/lib/Frontend/HLSL/HLSLRootSignature.cpp index 5351239b94b1e..9f8e2fb17e787 100644 --- a/llvm/lib/Frontend/HLSL/HLSLRootSignature.cpp +++ b/llvm/lib/Frontend/HLSL/HLSLRootSignature.cpp @@ -144,6 +144,33 @@ void DescriptorTableClause::dump(raw_ostream &OS) const { OS << ", flags = " << Flags << ")"; } +// Helper struct so that we can use the overloaded notation of std::visit +template <class... Ts> struct OverloadMethods : Ts... { + using Ts::operator()...; +}; + +template <class... Ts> OverloadMethods(Ts...) -> OverloadMethods<Ts...>; + +void dumpRootElements(raw_ostream &OS, ArrayRef<RootElement> Elements) { + OS << "RootElements{"; + bool First = true; + for (auto Element : Elements) { + if (!First) + OS << ","; + OS << " "; + First = false; + std::visit(OverloadMethods{ + [&OS](DescriptorTable Table) { + Table.dump(OS); + }, + [&OS](DescriptorTableClause Clause) { + Clause.dump(OS); + } + }, Element); + } + OS << "}"; +} + } // namespace rootsig } // namespace hlsl } // namespace llvm >From 403dbf8398f69a6d23faacc6782f72853e7194e0 Mon Sep 17 00:00:00 2001 From: Finn Plummer <canadienf...@gmail.com> Date: Fri, 2 May 2025 19:49:48 +0000 Subject: [PATCH 13/16] self-review: remove ast allocation --- clang/lib/Parse/ParseDeclCXX.cpp | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/clang/lib/Parse/ParseDeclCXX.cpp b/clang/lib/Parse/ParseDeclCXX.cpp index 8f1c86ec3bb3e..f1216331877ba 100644 --- a/clang/lib/Parse/ParseDeclCXX.cpp +++ b/clang/lib/Parse/ParseDeclCXX.cpp @@ -5369,20 +5369,13 @@ void Parser::ParseMicrosoftRootSignatureAttributeArgs(ParsedAttributes &Attrs) { 1); // offset 1 for '"' // Invoke the root signature parser to construct the in-memory constructs hlsl::RootSignatureLexer Lexer(Signature, SignatureLoc); - SmallVector<llvm::hlsl::rootsig::RootElement> Elements; - hlsl::RootSignatureParser Parser(Elements, Lexer, PP); + SmallVector<llvm::hlsl::rootsig::RootElement> RootElements; + hlsl::RootSignatureParser Parser(RootElements, Lexer, PP); if (Parser.parse()) { T.consumeClose(); return; } - // Allocate the root elements onto ASTContext - unsigned N = Elements.size(); - auto RootElements = MutableArrayRef<llvm::hlsl::rootsig::RootElement>( - ::new (Actions.getASTContext()) llvm::hlsl::rootsig::RootElement[N], N); - for (unsigned I = 0; I < N; ++I) - RootElements[I] = Elements[I]; - // Create the Root Signature auto *SignatureDecl = HLSLRootSignatureDecl::Create( Actions.getASTContext(), /*DeclContext=*/Actions.CurContext, >From bf8c671191f6e30cc288e90a72c276e842b1fba7 Mon Sep 17 00:00:00 2001 From: Finn Plummer <canadienf...@gmail.com> Date: Fri, 2 May 2025 19:58:14 +0000 Subject: [PATCH 14/16] clang-format --- llvm/lib/Frontend/HLSL/HLSLRootSignature.cpp | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/llvm/lib/Frontend/HLSL/HLSLRootSignature.cpp b/llvm/lib/Frontend/HLSL/HLSLRootSignature.cpp index 9f8e2fb17e787..8802bf3c1e42e 100644 --- a/llvm/lib/Frontend/HLSL/HLSLRootSignature.cpp +++ b/llvm/lib/Frontend/HLSL/HLSLRootSignature.cpp @@ -160,13 +160,9 @@ void dumpRootElements(raw_ostream &OS, ArrayRef<RootElement> Elements) { OS << " "; First = false; std::visit(OverloadMethods{ - [&OS](DescriptorTable Table) { - Table.dump(OS); - }, - [&OS](DescriptorTableClause Clause) { - Clause.dump(OS); - } - }, Element); + [&OS](DescriptorTable Table) { Table.dump(OS); }, + [&OS](DescriptorTableClause Clause) { Clause.dump(OS); }}, + Element); } OS << "}"; } >From a16dde11f2ec11bb8ccba359c246180702268b70 Mon Sep 17 00:00:00 2001 From: Finn Plummer <canadienf...@gmail.com> Date: Fri, 9 May 2025 16:59:52 +0000 Subject: [PATCH 15/16] review: remove forced copy --- llvm/lib/Frontend/HLSL/HLSLRootSignature.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/llvm/lib/Frontend/HLSL/HLSLRootSignature.cpp b/llvm/lib/Frontend/HLSL/HLSLRootSignature.cpp index 8802bf3c1e42e..893bcfe107587 100644 --- a/llvm/lib/Frontend/HLSL/HLSLRootSignature.cpp +++ b/llvm/lib/Frontend/HLSL/HLSLRootSignature.cpp @@ -154,7 +154,7 @@ template <class... Ts> OverloadMethods(Ts...) -> OverloadMethods<Ts...>; void dumpRootElements(raw_ostream &OS, ArrayRef<RootElement> Elements) { OS << "RootElements{"; bool First = true; - for (auto Element : Elements) { + for (const RootElement &Element : Elements) { if (!First) OS << ","; OS << " "; >From 5d397798e57d7eb7ce4cf68dd80a39e73103851b Mon Sep 17 00:00:00 2001 From: Finn Plummer <canadienf...@gmail.com> Date: Fri, 9 May 2025 17:00:34 +0000 Subject: [PATCH 16/16] review: simplify overloaded notation --- llvm/lib/Frontend/HLSL/HLSLRootSignature.cpp | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/llvm/lib/Frontend/HLSL/HLSLRootSignature.cpp b/llvm/lib/Frontend/HLSL/HLSLRootSignature.cpp index 893bcfe107587..6049f743475f1 100644 --- a/llvm/lib/Frontend/HLSL/HLSLRootSignature.cpp +++ b/llvm/lib/Frontend/HLSL/HLSLRootSignature.cpp @@ -144,14 +144,18 @@ void DescriptorTableClause::dump(raw_ostream &OS) const { OS << ", flags = " << Flags << ")"; } -// Helper struct so that we can use the overloaded notation of std::visit -template <class... Ts> struct OverloadMethods : Ts... { - using Ts::operator()...; +// Helper callable so that we can use the overloaded notation of std::visit +namespace { +struct ElementDumper { + raw_ostream &OS; + template <typename T> void operator()(const T &Element) const { + Element.dump(OS); + } }; - -template <class... Ts> OverloadMethods(Ts...) -> OverloadMethods<Ts...>; +} // namespace void dumpRootElements(raw_ostream &OS, ArrayRef<RootElement> Elements) { + ElementDumper Dumper{OS}; OS << "RootElements{"; bool First = true; for (const RootElement &Element : Elements) { @@ -159,10 +163,7 @@ void dumpRootElements(raw_ostream &OS, ArrayRef<RootElement> Elements) { OS << ","; OS << " "; First = false; - std::visit(OverloadMethods{ - [&OS](DescriptorTable Table) { Table.dump(OS); }, - [&OS](DescriptorTableClause Clause) { Clause.dump(OS); }}, - Element); + std::visit(Dumper, Element); } OS << "}"; } _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits