https://github.com/inbelic updated https://github.com/llvm/llvm-project/pull/147115
>From 564f6995f40d80acddbda1fce58ddec38613d9fa Mon Sep 17 00:00:00 2001 From: Finn Plummer <canadienf...@gmail.com> Date: Fri, 27 Jun 2025 18:36:38 +0000 Subject: [PATCH 01/15] nfc: introduce wrapper `RootSignatureElement` around `RootElement` to retain clang diag info --- .../clang/Parse/ParseHLSLRootSignature.h | 16 ++- clang/include/clang/Sema/SemaHLSL.h | 10 +- clang/lib/Parse/ParseDeclCXX.cpp | 2 +- clang/lib/Parse/ParseHLSLRootSignature.cpp | 14 +-- clang/lib/Sema/SemaHLSL.cpp | 6 +- .../Parse/ParseHLSLRootSignatureTest.cpp | 115 +++++++++--------- 6 files changed, 92 insertions(+), 71 deletions(-) diff --git a/clang/include/clang/Parse/ParseHLSLRootSignature.h b/clang/include/clang/Parse/ParseHLSLRootSignature.h index 007c3f7ba1e9c..41d3df97f9dfc 100644 --- a/clang/include/clang/Parse/ParseHLSLRootSignature.h +++ b/clang/include/clang/Parse/ParseHLSLRootSignature.h @@ -26,10 +26,22 @@ namespace clang { namespace hlsl { +// Introduce a wrapper struct around the underlying RootElement. This structure +// will retain extra clang diagnostic information that is not available in llvm. +struct RootSignatureElement { + RootSignatureElement(llvm::hlsl::rootsig::RootElement Element) + : Element(Element) {} + + const llvm::hlsl::rootsig::RootElement &getElement() const { return Element; } + +private: + llvm::hlsl::rootsig::RootElement Element; +}; + class RootSignatureParser { public: RootSignatureParser(llvm::dxbc::RootSignatureVersion Version, - SmallVector<llvm::hlsl::rootsig::RootElement> &Elements, + SmallVector<RootSignatureElement> &Elements, StringLiteral *Signature, Preprocessor &PP); /// Consumes tokens from the Lexer and constructs the in-memory @@ -201,7 +213,7 @@ class RootSignatureParser { private: llvm::dxbc::RootSignatureVersion Version; - SmallVector<llvm::hlsl::rootsig::RootElement> &Elements; + SmallVector<RootSignatureElement> &Elements; StringLiteral *Signature; RootSignatureLexer Lexer; Preprocessor &PP; diff --git a/clang/include/clang/Sema/SemaHLSL.h b/clang/include/clang/Sema/SemaHLSL.h index 7d7eae4db532c..1af706da702c2 100644 --- a/clang/include/clang/Sema/SemaHLSL.h +++ b/clang/include/clang/Sema/SemaHLSL.h @@ -32,6 +32,10 @@ class ParsedAttr; class Scope; class VarDecl; +namespace hlsl { +struct RootSignatureElement; +} + using llvm::dxil::ResourceClass; // FIXME: This can be hidden (as static function in SemaHLSL.cpp) once we no @@ -130,9 +134,9 @@ class SemaHLSL : public SemaBase { /// Creates the Root Signature decl of the parsed Root Signature elements /// onto the AST and push it onto current Scope - void ActOnFinishRootSignatureDecl( - SourceLocation Loc, IdentifierInfo *DeclIdent, - SmallVector<llvm::hlsl::rootsig::RootElement> &Elements); + void + ActOnFinishRootSignatureDecl(SourceLocation Loc, IdentifierInfo *DeclIdent, + ArrayRef<hlsl::RootSignatureElement> Elements); // Returns true when D is invalid and a diagnostic was produced bool handleRootSignatureDecl(HLSLRootSignatureDecl *D, SourceLocation Loc); diff --git a/clang/lib/Parse/ParseDeclCXX.cpp b/clang/lib/Parse/ParseDeclCXX.cpp index 59e6e0af4b5b0..9fe18b0dbcedb 100644 --- a/clang/lib/Parse/ParseDeclCXX.cpp +++ b/clang/lib/Parse/ParseDeclCXX.cpp @@ -4948,7 +4948,7 @@ void Parser::ParseHLSLRootSignatureAttributeArgs(ParsedAttributes &Attrs) { // signature string and construct the in-memory elements if (!Found) { // Invoke the root signature parser to construct the in-memory constructs - SmallVector<llvm::hlsl::rootsig::RootElement> RootElements; + SmallVector<hlsl::RootSignatureElement> RootElements; hlsl::RootSignatureParser Parser(getLangOpts().HLSLRootSigVer, RootElements, Signature, PP); if (Parser.parse()) { diff --git a/clang/lib/Parse/ParseHLSLRootSignature.cpp b/clang/lib/Parse/ParseHLSLRootSignature.cpp index dc5f6faefbab4..00a611416a86a 100644 --- a/clang/lib/Parse/ParseHLSLRootSignature.cpp +++ b/clang/lib/Parse/ParseHLSLRootSignature.cpp @@ -19,7 +19,7 @@ using TokenKind = RootSignatureToken::Kind; RootSignatureParser::RootSignatureParser( llvm::dxbc::RootSignatureVersion Version, - SmallVector<RootElement> &Elements, StringLiteral *Signature, + SmallVector<RootSignatureElement> &Elements, StringLiteral *Signature, Preprocessor &PP) : Version(Version), Elements(Elements), Signature(Signature), Lexer(Signature->getString()), PP(PP), CurToken(0) {} @@ -32,28 +32,28 @@ bool RootSignatureParser::parse() { auto Flags = parseRootFlags(); if (!Flags.has_value()) return true; - Elements.push_back(*Flags); + Elements.emplace_back(RootSignatureElement(*Flags)); } else if (tryConsumeExpectedToken(TokenKind::kw_RootConstants)) { auto Constants = parseRootConstants(); if (!Constants.has_value()) return true; - Elements.push_back(*Constants); + Elements.emplace_back(RootSignatureElement(*Constants)); } else if (tryConsumeExpectedToken(TokenKind::kw_DescriptorTable)) { auto Table = parseDescriptorTable(); if (!Table.has_value()) return true; - Elements.push_back(*Table); + Elements.emplace_back(RootSignatureElement(*Table)); } else if (tryConsumeExpectedToken( {TokenKind::kw_CBV, TokenKind::kw_SRV, TokenKind::kw_UAV})) { auto Descriptor = parseRootDescriptor(); if (!Descriptor.has_value()) return true; - Elements.push_back(*Descriptor); + Elements.emplace_back(RootSignatureElement(*Descriptor)); } else if (tryConsumeExpectedToken(TokenKind::kw_StaticSampler)) { auto Sampler = parseStaticSampler(); if (!Sampler.has_value()) return true; - Elements.push_back(*Sampler); + Elements.emplace_back(RootSignatureElement(*Sampler)); } // ',' denotes another element, otherwise, expected to be at end of stream @@ -248,7 +248,7 @@ std::optional<DescriptorTable> RootSignatureParser::parseDescriptorTable() { auto Clause = parseDescriptorTableClause(); if (!Clause.has_value()) return std::nullopt; - Elements.push_back(*Clause); + Elements.push_back(RootSignatureElement(*Clause)); Table.NumClauses++; } else if (tryConsumeExpectedToken(TokenKind::kw_visibility)) { // visibility = SHADER_VISIBILITY diff --git a/clang/lib/Sema/SemaHLSL.cpp b/clang/lib/Sema/SemaHLSL.cpp index cc50d23e0b998..e768b50339f21 100644 --- a/clang/lib/Sema/SemaHLSL.cpp +++ b/clang/lib/Sema/SemaHLSL.cpp @@ -1064,7 +1064,11 @@ SemaHLSL::ActOnStartRootSignatureDecl(StringRef Signature) { void SemaHLSL::ActOnFinishRootSignatureDecl( SourceLocation Loc, IdentifierInfo *DeclIdent, - SmallVector<llvm::hlsl::rootsig::RootElement> &Elements) { + ArrayRef<hlsl::RootSignatureElement> RootElements) { + + SmallVector<llvm::hlsl::rootsig::RootElement> Elements; + for (auto &RootSigElement : RootElements) + Elements.push_back(RootSigElement.getElement()); auto *SignatureDecl = HLSLRootSignatureDecl::Create( SemaRef.getASTContext(), /*DeclContext=*/SemaRef.CurContext, Loc, diff --git a/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp b/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp index e82dcadebba3f..2dd755aecce03 100644 --- a/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp +++ b/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp @@ -27,6 +27,7 @@ #include "gtest/gtest.h" using namespace clang; +using namespace clang::hlsl; using namespace llvm::hlsl::rootsig; namespace { @@ -135,7 +136,7 @@ TEST_F(ParseHLSLRootSignatureTest, ValidParseEmptyTest) { TrivialModuleLoader ModLoader; auto PP = createPP(Source, ModLoader); - SmallVector<RootElement> Elements; + SmallVector<RootSignatureElement> Elements; hlsl::RootSignatureParser Parser(RootSignatureVersion::V1_1, Elements, Signature, *PP); @@ -171,7 +172,7 @@ TEST_F(ParseHLSLRootSignatureTest, ValidParseDTClausesTest) { TrivialModuleLoader ModLoader; auto PP = createPP(Source, ModLoader); - SmallVector<RootElement> Elements; + SmallVector<RootSignatureElement> Elements; hlsl::RootSignatureParser Parser(RootSignatureVersion::V1_1, Elements, Signature, *PP); @@ -181,7 +182,7 @@ TEST_F(ParseHLSLRootSignatureTest, ValidParseDTClausesTest) { ASSERT_FALSE(Parser.parse()); // First Descriptor Table with 4 elements - RootElement Elem = Elements[0]; + RootElement Elem = Elements[0].getElement(); ASSERT_TRUE(std::holds_alternative<DescriptorTableClause>(Elem)); ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Type, ClauseType::CBuffer); ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Reg.ViewType, @@ -194,7 +195,7 @@ TEST_F(ParseHLSLRootSignatureTest, ValidParseDTClausesTest) { ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Flags, DescriptorRangeFlags::DataStaticWhileSetAtExecute); - Elem = Elements[1]; + Elem = Elements[1].getElement(); ASSERT_TRUE(std::holds_alternative<DescriptorTableClause>(Elem)); ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Type, ClauseType::SRV); ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Reg.ViewType, @@ -206,7 +207,7 @@ TEST_F(ParseHLSLRootSignatureTest, ValidParseDTClausesTest) { ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Flags, DescriptorRangeFlags::None); - Elem = Elements[2]; + Elem = Elements[2].getElement(); ASSERT_TRUE(std::holds_alternative<DescriptorTableClause>(Elem)); ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Type, ClauseType::Sampler); ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Reg.ViewType, @@ -219,7 +220,7 @@ TEST_F(ParseHLSLRootSignatureTest, ValidParseDTClausesTest) { ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Flags, DescriptorRangeFlags::None); - Elem = Elements[3]; + Elem = Elements[3].getElement(); ASSERT_TRUE(std::holds_alternative<DescriptorTableClause>(Elem)); ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Type, ClauseType::UAV); ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Reg.ViewType, @@ -239,14 +240,14 @@ TEST_F(ParseHLSLRootSignatureTest, ValidParseDTClausesTest) { ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Flags, ValidDescriptorRangeFlags); - Elem = Elements[4]; + Elem = Elements[4].getElement(); ASSERT_TRUE(std::holds_alternative<DescriptorTable>(Elem)); ASSERT_EQ(std::get<DescriptorTable>(Elem).NumClauses, (uint32_t)4); ASSERT_EQ(std::get<DescriptorTable>(Elem).Visibility, llvm::dxbc::ShaderVisibility::Pixel); // Empty Descriptor Table - Elem = Elements[5]; + Elem = Elements[5].getElement(); ASSERT_TRUE(std::holds_alternative<DescriptorTable>(Elem)); ASSERT_EQ(std::get<DescriptorTable>(Elem).NumClauses, 0u); ASSERT_EQ(std::get<DescriptorTable>(Elem).Visibility, @@ -276,7 +277,7 @@ TEST_F(ParseHLSLRootSignatureTest, ValidParseStaticSamplerTest) { TrivialModuleLoader ModLoader; auto PP = createPP(Source, ModLoader); - SmallVector<RootElement> Elements; + SmallVector<RootSignatureElement> Elements; hlsl::RootSignatureParser Parser(RootSignatureVersion::V1_1, Elements, Signature, *PP); @@ -288,7 +289,7 @@ TEST_F(ParseHLSLRootSignatureTest, ValidParseStaticSamplerTest) { ASSERT_EQ(Elements.size(), 2u); // Check default values are as expected - RootElement Elem = Elements[0]; + RootElement Elem = Elements[0].getElement(); ASSERT_TRUE(std::holds_alternative<StaticSampler>(Elem)); ASSERT_EQ(std::get<StaticSampler>(Elem).Reg.ViewType, RegisterType::SReg); ASSERT_EQ(std::get<StaticSampler>(Elem).Reg.Number, 0u); @@ -313,7 +314,7 @@ TEST_F(ParseHLSLRootSignatureTest, ValidParseStaticSamplerTest) { llvm::dxbc::ShaderVisibility::All); // Check values can be set as expected - Elem = Elements[1]; + Elem = Elements[1].getElement(); ASSERT_TRUE(std::holds_alternative<StaticSampler>(Elem)); ASSERT_EQ(std::get<StaticSampler>(Elem).Reg.ViewType, RegisterType::SReg); ASSERT_EQ(std::get<StaticSampler>(Elem).Reg.Number, 0u); @@ -363,7 +364,7 @@ TEST_F(ParseHLSLRootSignatureTest, ValidParseFloatsTest) { TrivialModuleLoader ModLoader; auto PP = createPP(Source, ModLoader); - SmallVector<RootElement> Elements; + SmallVector<RootSignatureElement> Elements; hlsl::RootSignatureParser Parser(RootSignatureVersion::V1_1, Elements, Signature, *PP); @@ -372,55 +373,55 @@ TEST_F(ParseHLSLRootSignatureTest, ValidParseFloatsTest) { ASSERT_FALSE(Parser.parse()); - RootElement Elem = Elements[0]; + RootElement Elem = Elements[0].getElement(); ASSERT_TRUE(std::holds_alternative<StaticSampler>(Elem)); ASSERT_FLOAT_EQ(std::get<StaticSampler>(Elem).MipLODBias, 0.f); - Elem = Elements[1]; + Elem = Elements[1].getElement(); ASSERT_TRUE(std::holds_alternative<StaticSampler>(Elem)); ASSERT_FLOAT_EQ(std::get<StaticSampler>(Elem).MipLODBias, 1.f); - Elem = Elements[2]; + Elem = Elements[2].getElement(); ASSERT_TRUE(std::holds_alternative<StaticSampler>(Elem)); ASSERT_FLOAT_EQ(std::get<StaticSampler>(Elem).MipLODBias, -1.f); - Elem = Elements[3]; + Elem = Elements[3].getElement(); ASSERT_TRUE(std::holds_alternative<StaticSampler>(Elem)); ASSERT_FLOAT_EQ(std::get<StaticSampler>(Elem).MipLODBias, 42.f); - Elem = Elements[4]; + Elem = Elements[4].getElement(); ASSERT_TRUE(std::holds_alternative<StaticSampler>(Elem)); ASSERT_FLOAT_EQ(std::get<StaticSampler>(Elem).MipLODBias, 4.2f); - Elem = Elements[5]; + Elem = Elements[5].getElement(); ASSERT_TRUE(std::holds_alternative<StaticSampler>(Elem)); ASSERT_FLOAT_EQ(std::get<StaticSampler>(Elem).MipLODBias, -.42f); - Elem = Elements[6]; + Elem = Elements[6].getElement(); ASSERT_TRUE(std::holds_alternative<StaticSampler>(Elem)); ASSERT_FLOAT_EQ(std::get<StaticSampler>(Elem).MipLODBias, 420.f); - Elem = Elements[7]; + Elem = Elements[7].getElement(); ASSERT_TRUE(std::holds_alternative<StaticSampler>(Elem)); ASSERT_FLOAT_EQ(std::get<StaticSampler>(Elem).MipLODBias, 0.000000000042f); - Elem = Elements[8]; + Elem = Elements[8].getElement(); ASSERT_TRUE(std::holds_alternative<StaticSampler>(Elem)); ASSERT_FLOAT_EQ(std::get<StaticSampler>(Elem).MipLODBias, 42.f); - Elem = Elements[9]; + Elem = Elements[9].getElement(); ASSERT_TRUE(std::holds_alternative<StaticSampler>(Elem)); ASSERT_FLOAT_EQ(std::get<StaticSampler>(Elem).MipLODBias, 4.2f); - Elem = Elements[10]; + Elem = Elements[10].getElement(); ASSERT_TRUE(std::holds_alternative<StaticSampler>(Elem)); ASSERT_FLOAT_EQ(std::get<StaticSampler>(Elem).MipLODBias, 420000000000.f); - Elem = Elements[11]; + Elem = Elements[11].getElement(); ASSERT_TRUE(std::holds_alternative<StaticSampler>(Elem)); ASSERT_FLOAT_EQ(std::get<StaticSampler>(Elem).MipLODBias, -2147483648.f); - Elem = Elements[12]; + Elem = Elements[12].getElement(); ASSERT_TRUE(std::holds_alternative<StaticSampler>(Elem)); ASSERT_FLOAT_EQ(std::get<StaticSampler>(Elem).MipLODBias, 2147483648.f); @@ -440,7 +441,7 @@ TEST_F(ParseHLSLRootSignatureTest, ValidSamplerFlagsTest) { TrivialModuleLoader ModLoader; auto PP = createPP(Source, ModLoader); - SmallVector<RootElement> Elements; + SmallVector<RootSignatureElement> Elements; hlsl::RootSignatureParser Parser(RootSignatureVersion::V1_1, Elements, Signature, *PP); @@ -449,7 +450,7 @@ TEST_F(ParseHLSLRootSignatureTest, ValidSamplerFlagsTest) { ASSERT_FALSE(Parser.parse()); - RootElement Elem = Elements[0]; + RootElement Elem = Elements[0].getElement(); ASSERT_TRUE(std::holds_alternative<DescriptorTableClause>(Elem)); ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Type, ClauseType::Sampler); auto ValidSamplerFlags = @@ -473,7 +474,7 @@ TEST_F(ParseHLSLRootSignatureTest, ValidParseRootConsantsTest) { TrivialModuleLoader ModLoader; auto PP = createPP(Source, ModLoader); - SmallVector<RootElement> Elements; + SmallVector<RootSignatureElement> Elements; hlsl::RootSignatureParser Parser(RootSignatureVersion::V1_1, Elements, Signature, *PP); @@ -484,7 +485,7 @@ TEST_F(ParseHLSLRootSignatureTest, ValidParseRootConsantsTest) { ASSERT_EQ(Elements.size(), 2u); - RootElement Elem = Elements[0]; + RootElement Elem = Elements[0].getElement(); ASSERT_TRUE(std::holds_alternative<RootConstants>(Elem)); ASSERT_EQ(std::get<RootConstants>(Elem).Num32BitConstants, 1u); ASSERT_EQ(std::get<RootConstants>(Elem).Reg.ViewType, RegisterType::BReg); @@ -493,7 +494,7 @@ TEST_F(ParseHLSLRootSignatureTest, ValidParseRootConsantsTest) { ASSERT_EQ(std::get<RootConstants>(Elem).Visibility, llvm::dxbc::ShaderVisibility::All); - Elem = Elements[1]; + Elem = Elements[1].getElement(); ASSERT_TRUE(std::holds_alternative<RootConstants>(Elem)); ASSERT_EQ(std::get<RootConstants>(Elem).Num32BitConstants, 4294967295u); ASSERT_EQ(std::get<RootConstants>(Elem).Reg.ViewType, RegisterType::BReg); @@ -532,7 +533,7 @@ TEST_F(ParseHLSLRootSignatureTest, ValidParseRootFlagsTest) { TrivialModuleLoader ModLoader; auto PP = createPP(Source, ModLoader); - SmallVector<RootElement> Elements; + SmallVector<RootSignatureElement> Elements; hlsl::RootSignatureParser Parser(RootSignatureVersion::V1_1, Elements, Signature, *PP); @@ -543,15 +544,15 @@ TEST_F(ParseHLSLRootSignatureTest, ValidParseRootFlagsTest) { ASSERT_EQ(Elements.size(), 3u); - RootElement Elem = Elements[0]; + RootElement Elem = Elements[0].getElement(); ASSERT_TRUE(std::holds_alternative<RootFlags>(Elem)); ASSERT_EQ(std::get<RootFlags>(Elem), RootFlags::None); - Elem = Elements[1]; + Elem = Elements[1].getElement(); ASSERT_TRUE(std::holds_alternative<RootFlags>(Elem)); ASSERT_EQ(std::get<RootFlags>(Elem), RootFlags::None); - Elem = Elements[2]; + Elem = Elements[2].getElement(); ASSERT_TRUE(std::holds_alternative<RootFlags>(Elem)); auto ValidRootFlags = RootFlags::AllowInputAssemblerInputLayout | RootFlags::DenyVertexShaderRootAccess | @@ -587,7 +588,7 @@ TEST_F(ParseHLSLRootSignatureTest, ValidParseRootDescriptorsTest) { TrivialModuleLoader ModLoader; auto PP = createPP(Source, ModLoader); - SmallVector<RootElement> Elements; + SmallVector<RootSignatureElement> Elements; hlsl::RootSignatureParser Parser(RootSignatureVersion::V1_1, Elements, Signature, *PP); @@ -598,7 +599,7 @@ TEST_F(ParseHLSLRootSignatureTest, ValidParseRootDescriptorsTest) { ASSERT_EQ(Elements.size(), 4u); - RootElement Elem = Elements[0]; + RootElement Elem = Elements[0].getElement(); ASSERT_TRUE(std::holds_alternative<RootDescriptor>(Elem)); ASSERT_EQ(std::get<RootDescriptor>(Elem).Type, DescriptorType::CBuffer); ASSERT_EQ(std::get<RootDescriptor>(Elem).Reg.ViewType, RegisterType::BReg); @@ -609,7 +610,7 @@ TEST_F(ParseHLSLRootSignatureTest, ValidParseRootDescriptorsTest) { ASSERT_EQ(std::get<RootDescriptor>(Elem).Flags, RootDescriptorFlags::DataStaticWhileSetAtExecute); - Elem = Elements[1]; + Elem = Elements[1].getElement(); ASSERT_TRUE(std::holds_alternative<RootDescriptor>(Elem)); ASSERT_EQ(std::get<RootDescriptor>(Elem).Type, DescriptorType::SRV); ASSERT_EQ(std::get<RootDescriptor>(Elem).Reg.ViewType, RegisterType::TReg); @@ -623,7 +624,7 @@ TEST_F(ParseHLSLRootSignatureTest, ValidParseRootDescriptorsTest) { RootDescriptorFlags::DataStatic; ASSERT_EQ(std::get<RootDescriptor>(Elem).Flags, ValidRootDescriptorFlags); - Elem = Elements[2]; + Elem = Elements[2].getElement(); ASSERT_TRUE(std::holds_alternative<RootDescriptor>(Elem)); ASSERT_EQ(std::get<RootDescriptor>(Elem).Type, DescriptorType::UAV); ASSERT_EQ(std::get<RootDescriptor>(Elem).Reg.ViewType, RegisterType::UReg); @@ -636,7 +637,7 @@ TEST_F(ParseHLSLRootSignatureTest, ValidParseRootDescriptorsTest) { ASSERT_EQ(std::get<RootDescriptor>(Elem).Flags, RootDescriptorFlags::DataVolatile); - Elem = Elements[3]; + Elem = Elements[3].getElement(); ASSERT_EQ(std::get<RootDescriptor>(Elem).Type, DescriptorType::CBuffer); ASSERT_EQ(std::get<RootDescriptor>(Elem).Reg.ViewType, RegisterType::BReg); ASSERT_EQ(std::get<RootDescriptor>(Elem).Reg.Number, 0u); @@ -663,7 +664,7 @@ TEST_F(ParseHLSLRootSignatureTest, ValidTrailingCommaTest) { TrivialModuleLoader ModLoader; auto PP = createPP(Source, ModLoader); - SmallVector<RootElement> Elements; + SmallVector<RootSignatureElement> Elements; hlsl::RootSignatureParser Parser(RootSignatureVersion::V1_1, Elements, Signature, *PP); @@ -837,7 +838,7 @@ TEST_F(ParseHLSLRootSignatureTest, InvalidParseUnexpectedTokenTest) { TrivialModuleLoader ModLoader; auto PP = createPP(Source, ModLoader); - SmallVector<RootElement> Elements; + SmallVector<RootSignatureElement> Elements; hlsl::RootSignatureParser Parser(RootSignatureVersion::V1_1, Elements, Signature, *PP); @@ -859,7 +860,7 @@ TEST_F(ParseHLSLRootSignatureTest, InvalidParseInvalidTokenTest) { TrivialModuleLoader ModLoader; auto PP = createPP(Source, ModLoader); - SmallVector<RootElement> Elements; + SmallVector<RootSignatureElement> Elements; hlsl::RootSignatureParser Parser(RootSignatureVersion::V1_1, Elements, Signature, *PP); @@ -881,7 +882,7 @@ TEST_F(ParseHLSLRootSignatureTest, InvalidParseUnexpectedEndOfStreamTest) { TrivialModuleLoader ModLoader; auto PP = createPP(Source, ModLoader); - SmallVector<RootElement> Elements; + SmallVector<RootSignatureElement> Elements; hlsl::RootSignatureParser Parser(RootSignatureVersion::V1_1, Elements, Signature, *PP); @@ -908,7 +909,7 @@ TEST_F(ParseHLSLRootSignatureTest, InvalidMissingDTParameterTest) { TrivialModuleLoader ModLoader; auto PP = createPP(Source, ModLoader); - SmallVector<RootElement> Elements; + SmallVector<RootSignatureElement> Elements; hlsl::RootSignatureParser Parser(RootSignatureVersion::V1_1, Elements, Signature, *PP); @@ -932,7 +933,7 @@ TEST_F(ParseHLSLRootSignatureTest, InvalidMissingRDParameterTest) { TrivialModuleLoader ModLoader; auto PP = createPP(Source, ModLoader); - SmallVector<RootElement> Elements; + SmallVector<RootSignatureElement> Elements; hlsl::RootSignatureParser Parser(RootSignatureVersion::V1_1, Elements, Signature, *PP); @@ -956,7 +957,7 @@ TEST_F(ParseHLSLRootSignatureTest, InvalidMissingRCParameterTest) { TrivialModuleLoader ModLoader; auto PP = createPP(Source, ModLoader); - SmallVector<RootElement> Elements; + SmallVector<RootSignatureElement> Elements; hlsl::RootSignatureParser Parser(RootSignatureVersion::V1_1, Elements, Signature, *PP); @@ -982,7 +983,7 @@ TEST_F(ParseHLSLRootSignatureTest, InvalidRepeatedMandatoryDTParameterTest) { TrivialModuleLoader ModLoader; auto PP = createPP(Source, ModLoader); - SmallVector<RootElement> Elements; + SmallVector<RootSignatureElement> Elements; hlsl::RootSignatureParser Parser(RootSignatureVersion::V1_1, Elements, Signature, *PP); @@ -1006,7 +1007,7 @@ TEST_F(ParseHLSLRootSignatureTest, InvalidRepeatedMandatoryRCParameterTest) { TrivialModuleLoader ModLoader; auto PP = createPP(Source, ModLoader); - SmallVector<RootElement> Elements; + SmallVector<RootSignatureElement> Elements; hlsl::RootSignatureParser Parser(RootSignatureVersion::V1_1, Elements, Signature, *PP); @@ -1032,7 +1033,7 @@ TEST_F(ParseHLSLRootSignatureTest, InvalidRepeatedOptionalDTParameterTest) { TrivialModuleLoader ModLoader; auto PP = createPP(Source, ModLoader); - SmallVector<RootElement> Elements; + SmallVector<RootSignatureElement> Elements; hlsl::RootSignatureParser Parser(RootSignatureVersion::V1_1, Elements, Signature, *PP); @@ -1060,7 +1061,7 @@ TEST_F(ParseHLSLRootSignatureTest, InvalidRepeatedOptionalRCParameterTest) { TrivialModuleLoader ModLoader; auto PP = createPP(Source, ModLoader); - SmallVector<RootElement> Elements; + SmallVector<RootSignatureElement> Elements; hlsl::RootSignatureParser Parser(RootSignatureVersion::V1_1, Elements, Signature, *PP); @@ -1085,7 +1086,7 @@ TEST_F(ParseHLSLRootSignatureTest, InvalidLexOverflowedNumberTest) { TrivialModuleLoader ModLoader; auto PP = createPP(Source, ModLoader); - SmallVector<RootElement> Elements; + SmallVector<RootSignatureElement> Elements; hlsl::RootSignatureParser Parser(RootSignatureVersion::V1_1, Elements, Signature, *PP); @@ -1109,7 +1110,7 @@ TEST_F(ParseHLSLRootSignatureTest, InvalidParseOverflowedNegativeNumberTest) { TrivialModuleLoader ModLoader; auto PP = createPP(Source, ModLoader); - SmallVector<RootElement> Elements; + SmallVector<RootSignatureElement> Elements; hlsl::RootSignatureParser Parser(RootSignatureVersion::V1_1, Elements, Signature, *PP); @@ -1132,7 +1133,7 @@ TEST_F(ParseHLSLRootSignatureTest, InvalidLexOverflowedFloatTest) { TrivialModuleLoader ModLoader; auto PP = createPP(Source, ModLoader); - SmallVector<RootElement> Elements; + SmallVector<RootSignatureElement> Elements; hlsl::RootSignatureParser Parser(RootSignatureVersion::V1_1, Elements, Signature, *PP); @@ -1155,7 +1156,7 @@ TEST_F(ParseHLSLRootSignatureTest, InvalidLexNegOverflowedFloatTest) { TrivialModuleLoader ModLoader; auto PP = createPP(Source, ModLoader); - SmallVector<RootElement> Elements; + SmallVector<RootSignatureElement> Elements; hlsl::RootSignatureParser Parser(RootSignatureVersion::V1_1, Elements, Signature, *PP); @@ -1178,7 +1179,7 @@ TEST_F(ParseHLSLRootSignatureTest, InvalidLexOverflowedDoubleTest) { TrivialModuleLoader ModLoader; auto PP = createPP(Source, ModLoader); - SmallVector<RootElement> Elements; + SmallVector<RootSignatureElement> Elements; hlsl::RootSignatureParser Parser(RootSignatureVersion::V1_1, Elements, Signature, *PP); @@ -1201,7 +1202,7 @@ TEST_F(ParseHLSLRootSignatureTest, InvalidLexUnderflowFloatTest) { TrivialModuleLoader ModLoader; auto PP = createPP(Source, ModLoader); - SmallVector<RootElement> Elements; + SmallVector<RootSignatureElement> Elements; hlsl::RootSignatureParser Parser(RootSignatureVersion::V1_1, Elements, Signature, *PP); @@ -1227,7 +1228,7 @@ TEST_F(ParseHLSLRootSignatureTest, InvalidNonZeroFlagsTest) { TrivialModuleLoader ModLoader; auto PP = createPP(Source, ModLoader); - SmallVector<RootElement> Elements; + SmallVector<RootSignatureElement> Elements; hlsl::RootSignatureParser Parser(RootSignatureVersion::V1_1, Elements, Signature, *PP); >From 337d241f24b6f64e1353b8a0f0c826f1ac1cd157 Mon Sep 17 00:00:00 2001 From: Finn Plummer <canadienf...@gmail.com> Date: Fri, 27 Jun 2025 18:53:10 +0000 Subject: [PATCH 02/15] let `RootSignatureElement` retain its source location --- .../clang/Parse/ParseHLSLRootSignature.h | 7 +++++-- clang/lib/Parse/ParseHLSLRootSignature.cpp | 18 ++++++++++++------ 2 files changed, 17 insertions(+), 8 deletions(-) diff --git a/clang/include/clang/Parse/ParseHLSLRootSignature.h b/clang/include/clang/Parse/ParseHLSLRootSignature.h index 41d3df97f9dfc..fc13bd4b59c6f 100644 --- a/clang/include/clang/Parse/ParseHLSLRootSignature.h +++ b/clang/include/clang/Parse/ParseHLSLRootSignature.h @@ -29,12 +29,15 @@ namespace hlsl { // Introduce a wrapper struct around the underlying RootElement. This structure // will retain extra clang diagnostic information that is not available in llvm. struct RootSignatureElement { - RootSignatureElement(llvm::hlsl::rootsig::RootElement Element) - : Element(Element) {} + RootSignatureElement(SourceLocation Loc, + llvm::hlsl::rootsig::RootElement Element) + : Loc(Loc), Element(Element) {} const llvm::hlsl::rootsig::RootElement &getElement() const { return Element; } + const SourceLocation &getLocation() const { return Loc; } private: + SourceLocation Loc; llvm::hlsl::rootsig::RootElement Element; }; diff --git a/clang/lib/Parse/ParseHLSLRootSignature.cpp b/clang/lib/Parse/ParseHLSLRootSignature.cpp index 00a611416a86a..5fa738b92da5f 100644 --- a/clang/lib/Parse/ParseHLSLRootSignature.cpp +++ b/clang/lib/Parse/ParseHLSLRootSignature.cpp @@ -29,31 +29,36 @@ bool RootSignatureParser::parse() { // end of the stream while (!peekExpectedToken(TokenKind::end_of_stream)) { if (tryConsumeExpectedToken(TokenKind::kw_RootFlags)) { + SourceLocation ElementLoc = getTokenLocation(CurToken); auto Flags = parseRootFlags(); if (!Flags.has_value()) return true; - Elements.emplace_back(RootSignatureElement(*Flags)); + Elements.emplace_back(RootSignatureElement(ElementLoc, *Flags)); } else if (tryConsumeExpectedToken(TokenKind::kw_RootConstants)) { + SourceLocation ElementLoc = getTokenLocation(CurToken); auto Constants = parseRootConstants(); if (!Constants.has_value()) return true; - Elements.emplace_back(RootSignatureElement(*Constants)); + Elements.emplace_back(RootSignatureElement(ElementLoc, *Constants)); } else if (tryConsumeExpectedToken(TokenKind::kw_DescriptorTable)) { + SourceLocation ElementLoc = getTokenLocation(CurToken); auto Table = parseDescriptorTable(); if (!Table.has_value()) return true; - Elements.emplace_back(RootSignatureElement(*Table)); + Elements.emplace_back(RootSignatureElement(ElementLoc, *Table)); } else if (tryConsumeExpectedToken( {TokenKind::kw_CBV, TokenKind::kw_SRV, TokenKind::kw_UAV})) { + SourceLocation ElementLoc = getTokenLocation(CurToken); auto Descriptor = parseRootDescriptor(); if (!Descriptor.has_value()) return true; - Elements.emplace_back(RootSignatureElement(*Descriptor)); + Elements.emplace_back(RootSignatureElement(ElementLoc, *Descriptor)); } else if (tryConsumeExpectedToken(TokenKind::kw_StaticSampler)) { + SourceLocation ElementLoc = getTokenLocation(CurToken); auto Sampler = parseStaticSampler(); if (!Sampler.has_value()) return true; - Elements.emplace_back(RootSignatureElement(*Sampler)); + Elements.emplace_back(RootSignatureElement(ElementLoc, *Sampler)); } // ',' denotes another element, otherwise, expected to be at end of stream @@ -245,10 +250,11 @@ std::optional<DescriptorTable> RootSignatureParser::parseDescriptorTable() { if (tryConsumeExpectedToken({TokenKind::kw_CBV, TokenKind::kw_SRV, TokenKind::kw_UAV, TokenKind::kw_Sampler})) { // DescriptorTableClause - CBV, SRV, UAV, or Sampler + SourceLocation ElementLoc = getTokenLocation(CurToken); auto Clause = parseDescriptorTableClause(); if (!Clause.has_value()) return std::nullopt; - Elements.push_back(RootSignatureElement(*Clause)); + Elements.push_back(RootSignatureElement(ElementLoc, *Clause)); Table.NumClauses++; } else if (tryConsumeExpectedToken(TokenKind::kw_visibility)) { // visibility = SHADER_VISIBILITY >From 580f21120a3a7ad7122da9fc8c1166605eed1588 Mon Sep 17 00:00:00 2001 From: Finn Plummer <canadienf...@gmail.com> Date: Fri, 27 Jun 2025 19:24:18 +0000 Subject: [PATCH 03/15] update resource range analysis to use retained source loc --- clang/include/clang/Sema/SemaHLSL.h | 4 +- clang/lib/Sema/SemaHLSL.cpp | 37 +++++++++++++++---- .../Frontend/HLSL/RootSignatureValidations.h | 3 ++ 3 files changed, 35 insertions(+), 9 deletions(-) diff --git a/clang/include/clang/Sema/SemaHLSL.h b/clang/include/clang/Sema/SemaHLSL.h index 1af706da702c2..5c944cbbd966b 100644 --- a/clang/include/clang/Sema/SemaHLSL.h +++ b/clang/include/clang/Sema/SemaHLSL.h @@ -139,7 +139,9 @@ class SemaHLSL : public SemaBase { ArrayRef<hlsl::RootSignatureElement> Elements); // Returns true when D is invalid and a diagnostic was produced - bool handleRootSignatureDecl(HLSLRootSignatureDecl *D, SourceLocation Loc); + bool + handleRootSignatureElements(ArrayRef<hlsl::RootSignatureElement> Elements, + SourceLocation Loc); void handleRootSignatureAttr(Decl *D, const ParsedAttr &AL); void handleNumThreadsAttr(Decl *D, const ParsedAttr &AL); void handleWaveSizeAttr(Decl *D, const ParsedAttr &AL); diff --git a/clang/lib/Sema/SemaHLSL.cpp b/clang/lib/Sema/SemaHLSL.cpp index e768b50339f21..233993557927e 100644 --- a/clang/lib/Sema/SemaHLSL.cpp +++ b/clang/lib/Sema/SemaHLSL.cpp @@ -28,6 +28,7 @@ #include "clang/Basic/SourceLocation.h" #include "clang/Basic/Specifiers.h" #include "clang/Basic/TargetInfo.h" +#include "clang/Parse/ParseHLSLRootSignature.h" #include "clang/Sema/Initialization.h" #include "clang/Sema/Lookup.h" #include "clang/Sema/ParsedAttr.h" @@ -1066,6 +1067,9 @@ void SemaHLSL::ActOnFinishRootSignatureDecl( SourceLocation Loc, IdentifierInfo *DeclIdent, ArrayRef<hlsl::RootSignatureElement> RootElements) { + if (handleRootSignatureElements(RootElements, Loc)) + return; + SmallVector<llvm::hlsl::rootsig::RootElement> Elements; for (auto &RootSigElement : RootElements) Elements.push_back(RootSigElement.getElement()); @@ -1074,21 +1078,24 @@ void SemaHLSL::ActOnFinishRootSignatureDecl( SemaRef.getASTContext(), /*DeclContext=*/SemaRef.CurContext, Loc, DeclIdent, SemaRef.getLangOpts().HLSLRootSigVer, Elements); - if (handleRootSignatureDecl(SignatureDecl, Loc)) - return; - SignatureDecl->setImplicit(); SemaRef.PushOnScopeChains(SignatureDecl, SemaRef.getCurScope()); } -bool SemaHLSL::handleRootSignatureDecl(HLSLRootSignatureDecl *D, - SourceLocation Loc) { +bool SemaHLSL::handleRootSignatureElements( + ArrayRef<hlsl::RootSignatureElement> Elements, SourceLocation Loc) { using RangeInfo = llvm::hlsl::rootsig::RangeInfo; using OverlappingRanges = llvm::hlsl::rootsig::OverlappingRanges; + // Introduce a mapping from the collected RangeInfos back to the + // RootSignatureElement that will retain its diagnostics info + llvm::DenseMap<size_t, const hlsl::RootSignatureElement *> InfoIndexMap; + size_t InfoIndex = 0; + // 1. Collect RangeInfos llvm::SmallVector<RangeInfo> Infos; - for (const llvm::hlsl::rootsig::RootElement &Elem : D->getRootElements()) { + for (const hlsl::RootSignatureElement &RootSigElem : Elements) { + const llvm::hlsl::rootsig::RootElement &Elem = RootSigElem.getElement(); if (const auto *Descriptor = std::get_if<llvm::hlsl::rootsig::RootDescriptor>(&Elem)) { RangeInfo Info; @@ -1099,6 +1106,9 @@ bool SemaHLSL::handleRootSignatureDecl(HLSLRootSignatureDecl *D, llvm::dxil::ResourceClass(llvm::to_underlying(Descriptor->Type)); Info.Space = Descriptor->Space; Info.Visibility = Descriptor->Visibility; + + Info.Index = InfoIndex++; + InfoIndexMap[Info.Index] = &RootSigElem; Infos.push_back(Info); } else if (const auto *Constants = std::get_if<llvm::hlsl::rootsig::RootConstants>(&Elem)) { @@ -1109,6 +1119,9 @@ bool SemaHLSL::handleRootSignatureDecl(HLSLRootSignatureDecl *D, Info.Class = llvm::dxil::ResourceClass::CBuffer; Info.Space = Constants->Space; Info.Visibility = Constants->Visibility; + + Info.Index = InfoIndex++; + InfoIndexMap[Info.Index] = &RootSigElem; Infos.push_back(Info); } else if (const auto *Sampler = std::get_if<llvm::hlsl::rootsig::StaticSampler>(&Elem)) { @@ -1119,6 +1132,9 @@ bool SemaHLSL::handleRootSignatureDecl(HLSLRootSignatureDecl *D, Info.Class = llvm::dxil::ResourceClass::Sampler; Info.Space = Sampler->Space; Info.Visibility = Sampler->Visibility; + + Info.Index = InfoIndex++; + InfoIndexMap[Info.Index] = &RootSigElem; Infos.push_back(Info); } else if (const auto *Clause = std::get_if<llvm::hlsl::rootsig::DescriptorTableClause>( @@ -1133,7 +1149,10 @@ bool SemaHLSL::handleRootSignatureDecl(HLSLRootSignatureDecl *D, Info.Class = Clause->Type; Info.Space = Clause->Space; + // Note: Clause does not hold the visibility this will need to + Info.Index = InfoIndex++; + InfoIndexMap[Info.Index] = &RootSigElem; Infos.push_back(Info); } else if (const auto *Table = std::get_if<llvm::hlsl::rootsig::DescriptorTable>(&Elem)) { @@ -1150,13 +1169,15 @@ bool SemaHLSL::handleRootSignatureDecl(HLSLRootSignatureDecl *D, } // Helper to report diagnostics - auto ReportOverlap = [this, Loc](OverlappingRanges Overlap) { + auto ReportOverlap = [this, &InfoIndexMap](OverlappingRanges Overlap) { const RangeInfo *Info = Overlap.A; const RangeInfo *OInfo = Overlap.B; auto CommonVis = Info->Visibility == llvm::dxbc::ShaderVisibility::All ? OInfo->Visibility : Info->Visibility; - this->Diag(Loc, diag::err_hlsl_resource_range_overlap) + const hlsl::RootSignatureElement *Elem = InfoIndexMap.at(Info->Index); + SourceLocation InfoLoc = Elem->getLocation(); + this->Diag(InfoLoc, diag::err_hlsl_resource_range_overlap) << llvm::to_underlying(Info->Class) << Info->LowerBound << /*unbounded=*/(Info->UpperBound == RangeInfo::Unbounded) << Info->UpperBound << llvm::to_underlying(OInfo->Class) diff --git a/llvm/include/llvm/Frontend/HLSL/RootSignatureValidations.h b/llvm/include/llvm/Frontend/HLSL/RootSignatureValidations.h index ea63a253b6661..e75679166336a 100644 --- a/llvm/include/llvm/Frontend/HLSL/RootSignatureValidations.h +++ b/llvm/include/llvm/Frontend/HLSL/RootSignatureValidations.h @@ -51,6 +51,9 @@ struct RangeInfo { llvm::dxil::ResourceClass Class; uint32_t Space; llvm::dxbc::ShaderVisibility Visibility; + + // The index retains its original position before being sorted by group. + size_t Index; }; class ResourceRange { >From f57b53600753a18a7f93589ece38b70d0349d659 Mon Sep 17 00:00:00 2001 From: Finn Plummer <canadienf...@gmail.com> Date: Fri, 27 Jun 2025 19:54:53 +0000 Subject: [PATCH 04/15] move wrapper definition to `SemaHLSL` - this struct needs to be accessible to both `Sema` and `Parse` and since `Parse` depends on `Sema` then we need to have it be included from there, so as to not introduce a circular dependency --- .../clang/Parse/ParseHLSLRootSignature.h | 16 +--------------- clang/include/clang/Sema/SemaHLSL.h | 19 +++++++++++++++++-- clang/lib/Sema/SemaHLSL.cpp | 1 - 3 files changed, 18 insertions(+), 18 deletions(-) diff --git a/clang/include/clang/Parse/ParseHLSLRootSignature.h b/clang/include/clang/Parse/ParseHLSLRootSignature.h index fc13bd4b59c6f..500e79a7ba574 100644 --- a/clang/include/clang/Parse/ParseHLSLRootSignature.h +++ b/clang/include/clang/Parse/ParseHLSLRootSignature.h @@ -17,6 +17,7 @@ #include "clang/Basic/DiagnosticParse.h" #include "clang/Lex/LexHLSLRootSignature.h" #include "clang/Lex/Preprocessor.h" +#include "clang/Sema/SemaHLSL.h" #include "llvm/ADT/SmallVector.h" #include "llvm/ADT/StringRef.h" @@ -26,21 +27,6 @@ namespace clang { namespace hlsl { -// Introduce a wrapper struct around the underlying RootElement. This structure -// will retain extra clang diagnostic information that is not available in llvm. -struct RootSignatureElement { - RootSignatureElement(SourceLocation Loc, - llvm::hlsl::rootsig::RootElement Element) - : Loc(Loc), Element(Element) {} - - const llvm::hlsl::rootsig::RootElement &getElement() const { return Element; } - const SourceLocation &getLocation() const { return Loc; } - -private: - SourceLocation Loc; - llvm::hlsl::rootsig::RootElement Element; -}; - class RootSignatureParser { public: RootSignatureParser(llvm::dxbc::RootSignatureVersion Version, diff --git a/clang/include/clang/Sema/SemaHLSL.h b/clang/include/clang/Sema/SemaHLSL.h index 5c944cbbd966b..910e0e640796b 100644 --- a/clang/include/clang/Sema/SemaHLSL.h +++ b/clang/include/clang/Sema/SemaHLSL.h @@ -33,8 +33,23 @@ class Scope; class VarDecl; namespace hlsl { -struct RootSignatureElement; -} + +// Introduce a wrapper struct around the underlying RootElement. This structure +// will retain extra clang diagnostic information that is not available in llvm. +struct RootSignatureElement { + RootSignatureElement(SourceLocation Loc, + llvm::hlsl::rootsig::RootElement Element) + : Loc(Loc), Element(Element) {} + + const llvm::hlsl::rootsig::RootElement &getElement() const { return Element; } + const SourceLocation &getLocation() const { return Loc; } + +private: + SourceLocation Loc; + llvm::hlsl::rootsig::RootElement Element; +}; + +} // namespace hlsl using llvm::dxil::ResourceClass; diff --git a/clang/lib/Sema/SemaHLSL.cpp b/clang/lib/Sema/SemaHLSL.cpp index 233993557927e..5f9148c34f292 100644 --- a/clang/lib/Sema/SemaHLSL.cpp +++ b/clang/lib/Sema/SemaHLSL.cpp @@ -28,7 +28,6 @@ #include "clang/Basic/SourceLocation.h" #include "clang/Basic/Specifiers.h" #include "clang/Basic/TargetInfo.h" -#include "clang/Parse/ParseHLSLRootSignature.h" #include "clang/Sema/Initialization.h" #include "clang/Sema/Lookup.h" #include "clang/Sema/ParsedAttr.h" >From 4eaf167809ae2f839dd77f8e894bda959a11ea3b Mon Sep 17 00:00:00 2001 From: Finn Plummer <canadienf...@gmail.com> Date: Thu, 3 Jul 2025 16:52:39 +0000 Subject: [PATCH 05/15] add note to denote where the other overlapping range is --- clang/include/clang/Basic/DiagnosticSemaKinds.td | 1 + clang/lib/Sema/SemaHLSL.cpp | 4 ++++ .../RootSignature-resource-ranges-err.hlsl | 14 ++++++++++++++ 3 files changed, 19 insertions(+) diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td index 934f4453f02b9..c22f2003e3cfa 100644 --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -13084,6 +13084,7 @@ def err_hlsl_resource_range_overlap: Error< "resource ranges %sub{subst_hlsl_format_ranges}0,1,2,3 and %sub{subst_hlsl_format_ranges}4,5,6,7 " "overlap within space = %8 and visibility = " "%select{All|Vertex|Hull|Domain|Geometry|Pixel|Amplification|Mesh}9">; +def note_hlsl_resource_range_here: Note<"overlapping resource range here">; // Layout randomization diagnostics. def err_non_designated_init_used : Error< diff --git a/clang/lib/Sema/SemaHLSL.cpp b/clang/lib/Sema/SemaHLSL.cpp index 5f9148c34f292..d50b70b533860 100644 --- a/clang/lib/Sema/SemaHLSL.cpp +++ b/clang/lib/Sema/SemaHLSL.cpp @@ -1183,6 +1183,10 @@ bool SemaHLSL::handleRootSignatureElements( << OInfo->LowerBound << /*unbounded=*/(OInfo->UpperBound == RangeInfo::Unbounded) << OInfo->UpperBound << Info->Space << CommonVis; + + const hlsl::RootSignatureElement *OElem = InfoIndexMap.at(OInfo->Index); + SourceLocation OInfoLoc = OElem->getLocation(); + this->Diag(OInfoLoc, diag::note_hlsl_resource_range_here); }; llvm::SmallVector<OverlappingRanges> Overlaps = diff --git a/clang/test/SemaHLSL/RootSignature-resource-ranges-err.hlsl b/clang/test/SemaHLSL/RootSignature-resource-ranges-err.hlsl index 4b3579d51818a..b98238dd43f7e 100644 --- a/clang/test/SemaHLSL/RootSignature-resource-ranges-err.hlsl +++ b/clang/test/SemaHLSL/RootSignature-resource-ranges-err.hlsl @@ -1,49 +1,61 @@ // RUN: %clang_cc1 -triple dxil-pc-shadermodel6.3-library -x hlsl -o - %s -verify +// expected-note@+2 {{overlapping resource range here}} // expected-error@+1 {{resource ranges b[42;42] and b[42;42] overlap within space = 0 and visibility = All}} [RootSignature("CBV(b42), CBV(b42)")] void bad_root_signature_0() {} +// expected-note@+2 {{overlapping resource range here}} // expected-error@+1 {{resource ranges t[0;0] and t[0;0] overlap within space = 3 and visibility = All}} [RootSignature("SRV(t0, space = 3), SRV(t0, space = 3)")] void bad_root_signature_1() {} +// expected-note@+2 {{overlapping resource range here}} // expected-error@+1 {{resource ranges u[0;0] and u[0;0] overlap within space = 0 and visibility = Pixel}} [RootSignature("UAV(u0, visibility = SHADER_VISIBILITY_PIXEL), UAV(u0, visibility = SHADER_VISIBILITY_PIXEL)")] void bad_root_signature_2() {} +// expected-note@+2 {{overlapping resource range here}} // expected-error@+1 {{resource ranges u[0;0] and u[0;0] overlap within space = 0 and visibility = Pixel}} [RootSignature("UAV(u0, visibility = SHADER_VISIBILITY_ALL), UAV(u0, visibility = SHADER_VISIBILITY_PIXEL)")] void bad_root_signature_3() {} +// expected-note@+2 {{overlapping resource range here}} // expected-error@+1 {{resource ranges u[0;0] and u[0;0] overlap within space = 0 and visibility = Pixel}} [RootSignature("UAV(u0, visibility = SHADER_VISIBILITY_PIXEL), UAV(u0, visibility = SHADER_VISIBILITY_ALL)")] void bad_root_signature_4() {} +// expected-note@+2 {{overlapping resource range here}} // expected-error@+1 {{resource ranges b[0;0] and b[0;0] overlap within space = 0 and visibility = All}} [RootSignature("RootConstants(num32BitConstants=4, b0), RootConstants(num32BitConstants=2, b0)")] void bad_root_signature_5() {} +// expected-note@+2 {{overlapping resource range here}} // expected-error@+1 {{resource ranges s[3;3] and s[3;3] overlap within space = 0 and visibility = All}} [RootSignature("StaticSampler(s3), StaticSampler(s3)")] void bad_root_signature_6() {} +// expected-note@+2 {{overlapping resource range here}} // expected-error@+1 {{resource ranges t[2;5] and t[0;3] overlap within space = 0 and visibility = All}} [RootSignature("DescriptorTable(SRV(t0, numDescriptors=4), SRV(t2, numDescriptors=4))")] void bad_root_signature_7() {} +// expected-note@+2 {{overlapping resource range here}} // expected-error@+1 {{resource ranges u[2;5] and u[0;unbounded) overlap within space = 0 and visibility = Hull}} [RootSignature("DescriptorTable(UAV(u0, numDescriptors=unbounded), visibility = SHADER_VISIBILITY_HULL), DescriptorTable(UAV(u2, numDescriptors=4))")] void bad_root_signature_8() {} +// expected-note@+2 {{overlapping resource range here}} // expected-error@+1 {{resource ranges b[0;2] and b[2;2] overlap within space = 0 and visibility = All}} [RootSignature("RootConstants(num32BitConstants=4, b2), DescriptorTable(CBV(b0, numDescriptors=3))")] void bad_root_signature_9() {} +// expected-note@+2 {{overlapping resource range here}} // expected-error@+1 {{resource ranges s[4;unbounded) and s[17;17] overlap within space = 0 and visibility = All}} [RootSignature("StaticSampler(s17), DescriptorTable(Sampler(s0, numDescriptors=3),Sampler(s4, numDescriptors=unbounded))")] void bad_root_signature_10() {} +// expected-note@+2 {{overlapping resource range here}} // expected-error@+1 {{resource ranges b[45;45] and b[4;unbounded) overlap within space = 0 and visibility = Geometry}} [RootSignature("DescriptorTable(CBV(b4, numDescriptors=unbounded)), CBV(b45, visibility = SHADER_VISIBILITY_GEOMETRY)")] void bad_root_signature_11() {} @@ -55,10 +67,12 @@ void bad_root_signature_11() {} " CBV(b0, numDescriptors = 8), " \ ")" +// expected-note@+2 {{overlapping resource range here}} // expected-error@+1 {{resource ranges b[0;7] and b[1;2] overlap within space = 0 and visibility = All}} [RootSignature(ReportFirstOverlap)] void bad_root_signature_12() {} +// expected-note@+2 {{overlapping resource range here}} // expected-error@+1 {{resource ranges s[2;2] and s[2;2] overlap within space = 0 and visibility = Vertex}} [RootSignature("StaticSampler(s2, visibility=SHADER_VISIBILITY_ALL), DescriptorTable(Sampler(s2), visibility=SHADER_VISIBILITY_VERTEX)")] void valid_root_signature_13() {} >From 17eb1e36f1dfa63b736967144936de0af451cd37 Mon Sep 17 00:00:00 2001 From: Finn Plummer <canadienf...@gmail.com> Date: Fri, 4 Jul 2025 17:04:39 +0000 Subject: [PATCH 06/15] add testcase to demonstrate source location --- .../RootSignature-resource-ranges-err.hlsl | 27 +++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/clang/test/SemaHLSL/RootSignature-resource-ranges-err.hlsl b/clang/test/SemaHLSL/RootSignature-resource-ranges-err.hlsl index b98238dd43f7e..a021722aea2a4 100644 --- a/clang/test/SemaHLSL/RootSignature-resource-ranges-err.hlsl +++ b/clang/test/SemaHLSL/RootSignature-resource-ranges-err.hlsl @@ -1,4 +1,5 @@ // RUN: %clang_cc1 -triple dxil-pc-shadermodel6.3-library -x hlsl -o - %s -verify +// RUN: not %clang_cc1 -triple dxil-pc-shadermodel6.3-library -x hlsl -o - %s 2>&1 | FileCheck %s // expected-note@+2 {{overlapping resource range here}} // expected-error@+1 {{resource ranges b[42;42] and b[42;42] overlap within space = 0 and visibility = All}} @@ -76,3 +77,29 @@ void bad_root_signature_12() {} // expected-error@+1 {{resource ranges s[2;2] and s[2;2] overlap within space = 0 and visibility = Vertex}} [RootSignature("StaticSampler(s2, visibility=SHADER_VISIBILITY_ALL), DescriptorTable(Sampler(s2), visibility=SHADER_VISIBILITY_VERTEX)")] void valid_root_signature_13() {} + +#define DemoNoteSourceLocations \ + "DescriptorTable( " \ + " CBV(b4, numDescriptors = 4), " \ + " SRV(t22, numDescriptors = 1), " \ + " UAV(u42, numDescriptors = 2), " \ + " CBV(b9, numDescriptors = 8), " \ + " SRV(t12, numDescriptors = 3), " \ + " UAV(u3, numDescriptors = 16), " \ + " SRV(t9, numDescriptors = 1), " \ + " CBV(b1, numDescriptors = 2), " \ + " SRV(t17, numDescriptors = 7), " \ + " UAV(u0, numDescriptors = 3), " \ + ")" + +// CHECK: note: expanded from macro 'DemoNoteSourceLocations' +// CHECK-NEXT: [[@LINE-5]] | " SRV(t17, numDescriptors = 7), " \ +// CHECK-NEXT: | ^ +// CHECK: note: expanded from macro 'DemoNoteSourceLocations' +// CHECK-NEXT: [[@LINE-15]] | " SRV(t22, numDescriptors = 1), " +// CHECK-NEXT: | ^ + +// expected-note@+2 {{overlapping resource range here}} +// expected-error@+1 {{resource ranges t[17;23] and t[22;22] overlap within space = 0 and visibility = All}} +[RootSignature(DemoNoteSourceLocations)] +void bad_root_signature_14() {} >From 7e341afd15b0fb4d9369263244f448e5eadf4d56 Mon Sep 17 00:00:00 2001 From: Finn Plummer <canadienf...@gmail.com> Date: Fri, 4 Jul 2025 18:03:28 +0000 Subject: [PATCH 07/15] rebase: clean-ups --- .../Parse/ParseHLSLRootSignatureTest.cpp | 32 +++++++++---------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp b/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp index 2dd755aecce03..3408959f428c5 100644 --- a/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp +++ b/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp @@ -697,7 +697,7 @@ TEST_F(ParseHLSLRootSignatureTest, ValidVersion10Test) { TrivialModuleLoader ModLoader; auto PP = createPP(Source, ModLoader); - SmallVector<RootElement> Elements; + SmallVector<RootSignatureElement> Elements; hlsl::RootSignatureParser Parser(RootSignatureVersion::V1_0, Elements, Signature, *PP); @@ -707,17 +707,17 @@ TEST_F(ParseHLSLRootSignatureTest, ValidVersion10Test) { ASSERT_FALSE(Parser.parse()); auto DefRootDescriptorFlag = llvm::dxbc::RootDescriptorFlags::DataVolatile; - RootElement Elem = Elements[0]; + RootElement Elem = Elements[0].getElement(); ASSERT_TRUE(std::holds_alternative<RootDescriptor>(Elem)); ASSERT_EQ(std::get<RootDescriptor>(Elem).Type, DescriptorType::CBuffer); ASSERT_EQ(std::get<RootDescriptor>(Elem).Flags, DefRootDescriptorFlag); - Elem = Elements[1]; + Elem = Elements[1].getElement(); ASSERT_TRUE(std::holds_alternative<RootDescriptor>(Elem)); ASSERT_EQ(std::get<RootDescriptor>(Elem).Type, DescriptorType::SRV); ASSERT_EQ(std::get<RootDescriptor>(Elem).Flags, DefRootDescriptorFlag); - Elem = Elements[2]; + Elem = Elements[2].getElement(); ASSERT_TRUE(std::holds_alternative<RootDescriptor>(Elem)); ASSERT_EQ(std::get<RootDescriptor>(Elem).Type, DescriptorType::UAV); ASSERT_EQ(std::get<RootDescriptor>(Elem).Flags, DefRootDescriptorFlag); @@ -725,22 +725,22 @@ TEST_F(ParseHLSLRootSignatureTest, ValidVersion10Test) { auto ValidNonSamplerFlags = llvm::dxbc::DescriptorRangeFlags::DescriptorsVolatile | llvm::dxbc::DescriptorRangeFlags::DataVolatile; - Elem = Elements[3]; + Elem = Elements[3].getElement(); ASSERT_TRUE(std::holds_alternative<DescriptorTableClause>(Elem)); ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Type, ClauseType::CBuffer); ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Flags, ValidNonSamplerFlags); - Elem = Elements[4]; + Elem = Elements[4].getElement(); ASSERT_TRUE(std::holds_alternative<DescriptorTableClause>(Elem)); ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Type, ClauseType::SRV); ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Flags, ValidNonSamplerFlags); - Elem = Elements[5]; + Elem = Elements[5].getElement(); ASSERT_TRUE(std::holds_alternative<DescriptorTableClause>(Elem)); ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Type, ClauseType::UAV); ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Flags, ValidNonSamplerFlags); - Elem = Elements[6]; + Elem = Elements[6].getElement(); ASSERT_TRUE(std::holds_alternative<DescriptorTableClause>(Elem)); ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Type, ClauseType::Sampler); ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Flags, @@ -770,7 +770,7 @@ TEST_F(ParseHLSLRootSignatureTest, ValidVersion11Test) { TrivialModuleLoader ModLoader; auto PP = createPP(Source, ModLoader); - SmallVector<RootElement> Elements; + SmallVector<RootSignatureElement> Elements; hlsl::RootSignatureParser Parser(RootSignatureVersion::V1_1, Elements, Signature, *PP); @@ -779,43 +779,43 @@ TEST_F(ParseHLSLRootSignatureTest, ValidVersion11Test) { ASSERT_FALSE(Parser.parse()); - RootElement Elem = Elements[0]; + RootElement Elem = Elements[0].getElement(); ASSERT_TRUE(std::holds_alternative<RootDescriptor>(Elem)); ASSERT_EQ(std::get<RootDescriptor>(Elem).Type, DescriptorType::CBuffer); ASSERT_EQ(std::get<RootDescriptor>(Elem).Flags, llvm::dxbc::RootDescriptorFlags::DataStaticWhileSetAtExecute); - Elem = Elements[1]; + Elem = Elements[1].getElement(); ASSERT_TRUE(std::holds_alternative<RootDescriptor>(Elem)); ASSERT_EQ(std::get<RootDescriptor>(Elem).Type, DescriptorType::SRV); ASSERT_EQ(std::get<RootDescriptor>(Elem).Flags, llvm::dxbc::RootDescriptorFlags::DataStaticWhileSetAtExecute); - Elem = Elements[2]; + Elem = Elements[2].getElement(); ASSERT_TRUE(std::holds_alternative<RootDescriptor>(Elem)); ASSERT_EQ(std::get<RootDescriptor>(Elem).Type, DescriptorType::UAV); ASSERT_EQ(std::get<RootDescriptor>(Elem).Flags, llvm::dxbc::RootDescriptorFlags::DataVolatile); - Elem = Elements[3]; + Elem = Elements[3].getElement(); ASSERT_TRUE(std::holds_alternative<DescriptorTableClause>(Elem)); ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Type, ClauseType::CBuffer); ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Flags, llvm::dxbc::DescriptorRangeFlags::DataStaticWhileSetAtExecute); - Elem = Elements[4]; + Elem = Elements[4].getElement(); ASSERT_TRUE(std::holds_alternative<DescriptorTableClause>(Elem)); ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Type, ClauseType::SRV); ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Flags, llvm::dxbc::DescriptorRangeFlags::DataStaticWhileSetAtExecute); - Elem = Elements[5]; + Elem = Elements[5].getElement(); ASSERT_TRUE(std::holds_alternative<DescriptorTableClause>(Elem)); ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Type, ClauseType::UAV); ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Flags, llvm::dxbc::DescriptorRangeFlags::DataVolatile); - Elem = Elements[6]; + Elem = Elements[6].getElement(); ASSERT_TRUE(std::holds_alternative<DescriptorTableClause>(Elem)); ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Type, ClauseType::Sampler); ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Flags, >From 7ae92c27d8b7e99ed988c3895e3cbd99cb13a38a Mon Sep 17 00:00:00 2001 From: Finn Plummer <canadienf...@gmail.com> Date: Fri, 4 Jul 2025 22:17:37 +0000 Subject: [PATCH 08/15] self-review: remove unused Loc --- clang/include/clang/Sema/SemaHLSL.h | 3 +-- clang/lib/Sema/SemaHLSL.cpp | 4 ++-- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/clang/include/clang/Sema/SemaHLSL.h b/clang/include/clang/Sema/SemaHLSL.h index 910e0e640796b..42abd29b01ea7 100644 --- a/clang/include/clang/Sema/SemaHLSL.h +++ b/clang/include/clang/Sema/SemaHLSL.h @@ -155,8 +155,7 @@ class SemaHLSL : public SemaBase { // Returns true when D is invalid and a diagnostic was produced bool - handleRootSignatureElements(ArrayRef<hlsl::RootSignatureElement> Elements, - SourceLocation Loc); + handleRootSignatureElements(ArrayRef<hlsl::RootSignatureElement> Elements); void handleRootSignatureAttr(Decl *D, const ParsedAttr &AL); void handleNumThreadsAttr(Decl *D, const ParsedAttr &AL); void handleWaveSizeAttr(Decl *D, const ParsedAttr &AL); diff --git a/clang/lib/Sema/SemaHLSL.cpp b/clang/lib/Sema/SemaHLSL.cpp index d50b70b533860..3f5c78f0f48e0 100644 --- a/clang/lib/Sema/SemaHLSL.cpp +++ b/clang/lib/Sema/SemaHLSL.cpp @@ -1066,7 +1066,7 @@ void SemaHLSL::ActOnFinishRootSignatureDecl( SourceLocation Loc, IdentifierInfo *DeclIdent, ArrayRef<hlsl::RootSignatureElement> RootElements) { - if (handleRootSignatureElements(RootElements, Loc)) + if (handleRootSignatureElements(RootElements)) return; SmallVector<llvm::hlsl::rootsig::RootElement> Elements; @@ -1082,7 +1082,7 @@ void SemaHLSL::ActOnFinishRootSignatureDecl( } bool SemaHLSL::handleRootSignatureElements( - ArrayRef<hlsl::RootSignatureElement> Elements, SourceLocation Loc) { + ArrayRef<hlsl::RootSignatureElement> Elements) { using RangeInfo = llvm::hlsl::rootsig::RangeInfo; using OverlappingRanges = llvm::hlsl::rootsig::OverlappingRanges; >From f5608e2d7b606c69a03a8895f03dc0e6c208709d Mon Sep 17 00:00:00 2001 From: Finn Plummer <canadienf...@gmail.com> Date: Tue, 8 Jul 2025 18:23:33 +0000 Subject: [PATCH 09/15] nfc, rebase: update newly added tests --- clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp b/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp index 3408959f428c5..5b1ab149f4497 100644 --- a/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp +++ b/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp @@ -1253,7 +1253,7 @@ TEST_F(ParseHLSLRootSignatureTest, InvalidRootElementMissingCommaTest) { TrivialModuleLoader ModLoader; auto PP = createPP(Source, ModLoader); - SmallVector<RootElement> Elements; + SmallVector<RootSignatureElement> Elements; hlsl::RootSignatureParser Parser(RootSignatureVersion::V1_1, Elements, Signature, *PP); @@ -1280,7 +1280,7 @@ TEST_F(ParseHLSLRootSignatureTest, InvalidDescriptorTableMissingCommaTest) { TrivialModuleLoader ModLoader; auto PP = createPP(Source, ModLoader); - SmallVector<RootElement> Elements; + SmallVector<RootSignatureElement> Elements; hlsl::RootSignatureParser Parser(RootSignatureVersion::V1_1, Elements, Signature, *PP); @@ -1307,7 +1307,7 @@ TEST_F(ParseHLSLRootSignatureTest, InvalidRootConstantParamsCommaTest) { TrivialModuleLoader ModLoader; auto PP = createPP(Source, ModLoader); - SmallVector<RootElement> Elements; + SmallVector<RootSignatureElement> Elements; hlsl::RootSignatureParser Parser(RootSignatureVersion::V1_1, Elements, Signature, *PP); @@ -1334,7 +1334,7 @@ TEST_F(ParseHLSLRootSignatureTest, InvalidRootDescriptorParamsCommaTest) { TrivialModuleLoader ModLoader; auto PP = createPP(Source, ModLoader); - SmallVector<RootElement> Elements; + SmallVector<RootSignatureElement> Elements; hlsl::RootSignatureParser Parser(RootSignatureVersion::V1_1, Elements, Signature, *PP); @@ -1363,7 +1363,7 @@ TEST_F(ParseHLSLRootSignatureTest, InvalidDescriptorClauseParamsCommaTest) { TrivialModuleLoader ModLoader; auto PP = createPP(Source, ModLoader); - SmallVector<RootElement> Elements; + SmallVector<RootSignatureElement> Elements; hlsl::RootSignatureParser Parser(RootSignatureVersion::V1_1, Elements, Signature, *PP); @@ -1390,7 +1390,7 @@ TEST_F(ParseHLSLRootSignatureTest, InvalidStaticSamplerCommaTest) { TrivialModuleLoader ModLoader; auto PP = createPP(Source, ModLoader); - SmallVector<RootElement> Elements; + SmallVector<RootSignatureElement> Elements; hlsl::RootSignatureParser Parser(RootSignatureVersion::V1_1, Elements, Signature, *PP); >From e7b5a8acc22eaca851781c98d9ae9569adaf966f Mon Sep 17 00:00:00 2001 From: Finn Plummer <canadienf...@gmail.com> Date: Tue, 8 Jul 2025 18:38:01 +0000 Subject: [PATCH 10/15] review: use emplace_back --- clang/lib/Parse/ParseHLSLRootSignature.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clang/lib/Parse/ParseHLSLRootSignature.cpp b/clang/lib/Parse/ParseHLSLRootSignature.cpp index 5fa738b92da5f..6c27ea22a2167 100644 --- a/clang/lib/Parse/ParseHLSLRootSignature.cpp +++ b/clang/lib/Parse/ParseHLSLRootSignature.cpp @@ -254,7 +254,7 @@ std::optional<DescriptorTable> RootSignatureParser::parseDescriptorTable() { auto Clause = parseDescriptorTableClause(); if (!Clause.has_value()) return std::nullopt; - Elements.push_back(RootSignatureElement(ElementLoc, *Clause)); + Elements.emplace_back(RootSignatureElement(ElementLoc, *Clause)); Table.NumClauses++; } else if (tryConsumeExpectedToken(TokenKind::kw_visibility)) { // visibility = SHADER_VISIBILITY >From 41eed5896634b13da9d4667c656d053dbb259da7 Mon Sep 17 00:00:00 2001 From: Finn Plummer <canadienf...@gmail.com> Date: Tue, 8 Jul 2025 18:43:10 +0000 Subject: [PATCH 11/15] review: remove redundant Index to Element mapping --- clang/lib/Sema/SemaHLSL.cpp | 27 +++++++++++---------------- 1 file changed, 11 insertions(+), 16 deletions(-) diff --git a/clang/lib/Sema/SemaHLSL.cpp b/clang/lib/Sema/SemaHLSL.cpp index 3f5c78f0f48e0..fd41896821986 100644 --- a/clang/lib/Sema/SemaHLSL.cpp +++ b/clang/lib/Sema/SemaHLSL.cpp @@ -1086,9 +1086,8 @@ bool SemaHLSL::handleRootSignatureElements( using RangeInfo = llvm::hlsl::rootsig::RangeInfo; using OverlappingRanges = llvm::hlsl::rootsig::OverlappingRanges; - // Introduce a mapping from the collected RangeInfos back to the - // RootSignatureElement that will retain its diagnostics info - llvm::DenseMap<size_t, const hlsl::RootSignatureElement *> InfoIndexMap; + // Each RangeInfo will contain an index back to its associated + // RootSignatureElement in our Elements ArrayRef size_t InfoIndex = 0; // 1. Collect RangeInfos @@ -1106,8 +1105,7 @@ bool SemaHLSL::handleRootSignatureElements( Info.Space = Descriptor->Space; Info.Visibility = Descriptor->Visibility; - Info.Index = InfoIndex++; - InfoIndexMap[Info.Index] = &RootSigElem; + Info.Index = InfoIndex; Infos.push_back(Info); } else if (const auto *Constants = std::get_if<llvm::hlsl::rootsig::RootConstants>(&Elem)) { @@ -1119,8 +1117,7 @@ bool SemaHLSL::handleRootSignatureElements( Info.Space = Constants->Space; Info.Visibility = Constants->Visibility; - Info.Index = InfoIndex++; - InfoIndexMap[Info.Index] = &RootSigElem; + Info.Index = InfoIndex; Infos.push_back(Info); } else if (const auto *Sampler = std::get_if<llvm::hlsl::rootsig::StaticSampler>(&Elem)) { @@ -1132,8 +1129,7 @@ bool SemaHLSL::handleRootSignatureElements( Info.Space = Sampler->Space; Info.Visibility = Sampler->Visibility; - Info.Index = InfoIndex++; - InfoIndexMap[Info.Index] = &RootSigElem; + Info.Index = InfoIndex; Infos.push_back(Info); } else if (const auto *Clause = std::get_if<llvm::hlsl::rootsig::DescriptorTableClause>( @@ -1150,8 +1146,7 @@ bool SemaHLSL::handleRootSignatureElements( Info.Space = Clause->Space; // Note: Clause does not hold the visibility this will need to - Info.Index = InfoIndex++; - InfoIndexMap[Info.Index] = &RootSigElem; + Info.Index = InfoIndex; Infos.push_back(Info); } else if (const auto *Table = std::get_if<llvm::hlsl::rootsig::DescriptorTable>(&Elem)) { @@ -1165,17 +1160,18 @@ bool SemaHLSL::handleRootSignatureElements( for (RangeInfo &Info : TableInfos) Info.Visibility = Table->Visibility; } + + InfoIndex++; } // Helper to report diagnostics - auto ReportOverlap = [this, &InfoIndexMap](OverlappingRanges Overlap) { + auto ReportOverlap = [this, &Elements](OverlappingRanges Overlap) { const RangeInfo *Info = Overlap.A; const RangeInfo *OInfo = Overlap.B; auto CommonVis = Info->Visibility == llvm::dxbc::ShaderVisibility::All ? OInfo->Visibility : Info->Visibility; - const hlsl::RootSignatureElement *Elem = InfoIndexMap.at(Info->Index); - SourceLocation InfoLoc = Elem->getLocation(); + SourceLocation InfoLoc = Elements[Info->Index].getLocation(); this->Diag(InfoLoc, diag::err_hlsl_resource_range_overlap) << llvm::to_underlying(Info->Class) << Info->LowerBound << /*unbounded=*/(Info->UpperBound == RangeInfo::Unbounded) @@ -1184,8 +1180,7 @@ bool SemaHLSL::handleRootSignatureElements( << /*unbounded=*/(OInfo->UpperBound == RangeInfo::Unbounded) << OInfo->UpperBound << Info->Space << CommonVis; - const hlsl::RootSignatureElement *OElem = InfoIndexMap.at(OInfo->Index); - SourceLocation OInfoLoc = OElem->getLocation(); + SourceLocation OInfoLoc = Elements[OInfo->Index].getLocation(); this->Diag(OInfoLoc, diag::note_hlsl_resource_range_here); }; >From 7a4680f13eea7e53c639d1b37dc72a6c9978b9bb Mon Sep 17 00:00:00 2001 From: Finn Plummer <canadienf...@gmail.com> Date: Tue, 8 Jul 2025 18:52:22 +0000 Subject: [PATCH 12/15] review: re-order notes to displayed order --- .../RootSignature-resource-ranges-err.hlsl | 60 +++++++++---------- 1 file changed, 30 insertions(+), 30 deletions(-) diff --git a/clang/test/SemaHLSL/RootSignature-resource-ranges-err.hlsl b/clang/test/SemaHLSL/RootSignature-resource-ranges-err.hlsl index a021722aea2a4..15a2707e35dd2 100644 --- a/clang/test/SemaHLSL/RootSignature-resource-ranges-err.hlsl +++ b/clang/test/SemaHLSL/RootSignature-resource-ranges-err.hlsl @@ -1,63 +1,63 @@ // RUN: %clang_cc1 -triple dxil-pc-shadermodel6.3-library -x hlsl -o - %s -verify // RUN: not %clang_cc1 -triple dxil-pc-shadermodel6.3-library -x hlsl -o - %s 2>&1 | FileCheck %s -// expected-note@+2 {{overlapping resource range here}} -// expected-error@+1 {{resource ranges b[42;42] and b[42;42] overlap within space = 0 and visibility = All}} +// expected-error@+2 {{resource ranges b[42;42] and b[42;42] overlap within space = 0 and visibility = All}} +// expected-note@+1 {{overlapping resource range here}} [RootSignature("CBV(b42), CBV(b42)")] void bad_root_signature_0() {} -// expected-note@+2 {{overlapping resource range here}} -// expected-error@+1 {{resource ranges t[0;0] and t[0;0] overlap within space = 3 and visibility = All}} +// expected-error@+2 {{resource ranges t[0;0] and t[0;0] overlap within space = 3 and visibility = All}} +// expected-note@+1 {{overlapping resource range here}} [RootSignature("SRV(t0, space = 3), SRV(t0, space = 3)")] void bad_root_signature_1() {} -// expected-note@+2 {{overlapping resource range here}} -// expected-error@+1 {{resource ranges u[0;0] and u[0;0] overlap within space = 0 and visibility = Pixel}} +// expected-error@+2 {{resource ranges u[0;0] and u[0;0] overlap within space = 0 and visibility = Pixel}} +// expected-note@+1 {{overlapping resource range here}} [RootSignature("UAV(u0, visibility = SHADER_VISIBILITY_PIXEL), UAV(u0, visibility = SHADER_VISIBILITY_PIXEL)")] void bad_root_signature_2() {} -// expected-note@+2 {{overlapping resource range here}} -// expected-error@+1 {{resource ranges u[0;0] and u[0;0] overlap within space = 0 and visibility = Pixel}} +// expected-error@+2 {{resource ranges u[0;0] and u[0;0] overlap within space = 0 and visibility = Pixel}} +// expected-note@+1 {{overlapping resource range here}} [RootSignature("UAV(u0, visibility = SHADER_VISIBILITY_ALL), UAV(u0, visibility = SHADER_VISIBILITY_PIXEL)")] void bad_root_signature_3() {} -// expected-note@+2 {{overlapping resource range here}} -// expected-error@+1 {{resource ranges u[0;0] and u[0;0] overlap within space = 0 and visibility = Pixel}} +// expected-error@+2 {{resource ranges u[0;0] and u[0;0] overlap within space = 0 and visibility = Pixel}} +// expected-note@+1 {{overlapping resource range here}} [RootSignature("UAV(u0, visibility = SHADER_VISIBILITY_PIXEL), UAV(u0, visibility = SHADER_VISIBILITY_ALL)")] void bad_root_signature_4() {} -// expected-note@+2 {{overlapping resource range here}} -// expected-error@+1 {{resource ranges b[0;0] and b[0;0] overlap within space = 0 and visibility = All}} +// expected-error@+2 {{resource ranges b[0;0] and b[0;0] overlap within space = 0 and visibility = All}} +// expected-note@+1 {{overlapping resource range here}} [RootSignature("RootConstants(num32BitConstants=4, b0), RootConstants(num32BitConstants=2, b0)")] void bad_root_signature_5() {} -// expected-note@+2 {{overlapping resource range here}} -// expected-error@+1 {{resource ranges s[3;3] and s[3;3] overlap within space = 0 and visibility = All}} +// expected-error@+2 {{resource ranges s[3;3] and s[3;3] overlap within space = 0 and visibility = All}} +// expected-note@+1 {{overlapping resource range here}} [RootSignature("StaticSampler(s3), StaticSampler(s3)")] void bad_root_signature_6() {} -// expected-note@+2 {{overlapping resource range here}} -// expected-error@+1 {{resource ranges t[2;5] and t[0;3] overlap within space = 0 and visibility = All}} +// expected-error@+2 {{resource ranges t[2;5] and t[0;3] overlap within space = 0 and visibility = All}} +// expected-note@+1 {{overlapping resource range here}} [RootSignature("DescriptorTable(SRV(t0, numDescriptors=4), SRV(t2, numDescriptors=4))")] void bad_root_signature_7() {} -// expected-note@+2 {{overlapping resource range here}} -// expected-error@+1 {{resource ranges u[2;5] and u[0;unbounded) overlap within space = 0 and visibility = Hull}} +// expected-error@+2 {{resource ranges u[2;5] and u[0;unbounded) overlap within space = 0 and visibility = Hull}} +// expected-note@+1 {{overlapping resource range here}} [RootSignature("DescriptorTable(UAV(u0, numDescriptors=unbounded), visibility = SHADER_VISIBILITY_HULL), DescriptorTable(UAV(u2, numDescriptors=4))")] void bad_root_signature_8() {} -// expected-note@+2 {{overlapping resource range here}} -// expected-error@+1 {{resource ranges b[0;2] and b[2;2] overlap within space = 0 and visibility = All}} +// expected-error@+2 {{resource ranges b[0;2] and b[2;2] overlap within space = 0 and visibility = All}} +// expected-note@+1 {{overlapping resource range here}} [RootSignature("RootConstants(num32BitConstants=4, b2), DescriptorTable(CBV(b0, numDescriptors=3))")] void bad_root_signature_9() {} -// expected-note@+2 {{overlapping resource range here}} -// expected-error@+1 {{resource ranges s[4;unbounded) and s[17;17] overlap within space = 0 and visibility = All}} +// expected-error@+2 {{resource ranges s[4;unbounded) and s[17;17] overlap within space = 0 and visibility = All}} +// expected-note@+1 {{overlapping resource range here}} [RootSignature("StaticSampler(s17), DescriptorTable(Sampler(s0, numDescriptors=3),Sampler(s4, numDescriptors=unbounded))")] void bad_root_signature_10() {} -// expected-note@+2 {{overlapping resource range here}} -// expected-error@+1 {{resource ranges b[45;45] and b[4;unbounded) overlap within space = 0 and visibility = Geometry}} +// expected-error@+2 {{resource ranges b[45;45] and b[4;unbounded) overlap within space = 0 and visibility = Geometry}} +// expected-note@+1 {{overlapping resource range here}} [RootSignature("DescriptorTable(CBV(b4, numDescriptors=unbounded)), CBV(b45, visibility = SHADER_VISIBILITY_GEOMETRY)")] void bad_root_signature_11() {} @@ -68,13 +68,13 @@ void bad_root_signature_11() {} " CBV(b0, numDescriptors = 8), " \ ")" -// expected-note@+2 {{overlapping resource range here}} -// expected-error@+1 {{resource ranges b[0;7] and b[1;2] overlap within space = 0 and visibility = All}} +// expected-error@+2 {{resource ranges b[0;7] and b[1;2] overlap within space = 0 and visibility = All}} +// expected-note@+1 {{overlapping resource range here}} [RootSignature(ReportFirstOverlap)] void bad_root_signature_12() {} -// expected-note@+2 {{overlapping resource range here}} -// expected-error@+1 {{resource ranges s[2;2] and s[2;2] overlap within space = 0 and visibility = Vertex}} +// expected-error@+2 {{resource ranges s[2;2] and s[2;2] overlap within space = 0 and visibility = Vertex}} +// expected-note@+1 {{overlapping resource range here}} [RootSignature("StaticSampler(s2, visibility=SHADER_VISIBILITY_ALL), DescriptorTable(Sampler(s2), visibility=SHADER_VISIBILITY_VERTEX)")] void valid_root_signature_13() {} @@ -99,7 +99,7 @@ void valid_root_signature_13() {} // CHECK-NEXT: [[@LINE-15]] | " SRV(t22, numDescriptors = 1), " // CHECK-NEXT: | ^ -// expected-note@+2 {{overlapping resource range here}} -// expected-error@+1 {{resource ranges t[17;23] and t[22;22] overlap within space = 0 and visibility = All}} +// expected-error@+2 {{resource ranges t[17;23] and t[22;22] overlap within space = 0 and visibility = All}} +// expected-note@+1 {{overlapping resource range here}} [RootSignature(DemoNoteSourceLocations)] void bad_root_signature_14() {} >From 9c71fb6ae107dcd825d32116772f3d6e807d581e Mon Sep 17 00:00:00 2001 From: Finn Plummer <canadienf...@gmail.com> Date: Tue, 8 Jul 2025 18:59:19 +0000 Subject: [PATCH 13/15] self-review: fix test command-line --- clang/test/SemaHLSL/RootSignature-resource-ranges-err.hlsl | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/clang/test/SemaHLSL/RootSignature-resource-ranges-err.hlsl b/clang/test/SemaHLSL/RootSignature-resource-ranges-err.hlsl index 15a2707e35dd2..6659ada0f2d9d 100644 --- a/clang/test/SemaHLSL/RootSignature-resource-ranges-err.hlsl +++ b/clang/test/SemaHLSL/RootSignature-resource-ranges-err.hlsl @@ -1,5 +1,5 @@ -// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.3-library -x hlsl -o - %s -verify -// RUN: not %clang_cc1 -triple dxil-pc-shadermodel6.3-library -x hlsl -o - %s 2>&1 | FileCheck %s +// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.3-library -x hlsl -fsyntax-only %s -verify +// RUN: not %clang_cc1 -triple dxil-pc-shadermodel6.3-library -x hlsl -fsyntax-only %s 2>&1 | FileCheck %s // expected-error@+2 {{resource ranges b[42;42] and b[42;42] overlap within space = 0 and visibility = All}} // expected-note@+1 {{overlapping resource range here}} >From 159ce712fd650609c55bdea8d8dbcbc01a431a4a Mon Sep 17 00:00:00 2001 From: Finn Plummer <canadienf...@gmail.com> Date: Tue, 8 Jul 2025 19:00:45 +0000 Subject: [PATCH 14/15] self-review: add column to test check --- clang/test/SemaHLSL/RootSignature-resource-ranges-err.hlsl | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/clang/test/SemaHLSL/RootSignature-resource-ranges-err.hlsl b/clang/test/SemaHLSL/RootSignature-resource-ranges-err.hlsl index 6659ada0f2d9d..61f122e080440 100644 --- a/clang/test/SemaHLSL/RootSignature-resource-ranges-err.hlsl +++ b/clang/test/SemaHLSL/RootSignature-resource-ranges-err.hlsl @@ -92,10 +92,10 @@ void valid_root_signature_13() {} " UAV(u0, numDescriptors = 3), " \ ")" -// CHECK: note: expanded from macro 'DemoNoteSourceLocations' +// CHECK: [[@LINE-4]]:5: note: expanded from macro 'DemoNoteSourceLocations' // CHECK-NEXT: [[@LINE-5]] | " SRV(t17, numDescriptors = 7), " \ // CHECK-NEXT: | ^ -// CHECK: note: expanded from macro 'DemoNoteSourceLocations' +// CHECK: [[@LINE-14]]:5: note: expanded from macro 'DemoNoteSourceLocations' // CHECK-NEXT: [[@LINE-15]] | " SRV(t22, numDescriptors = 1), " // CHECK-NEXT: | ^ >From 1c983b5d75b7feed71306db111e1108c6bf88615 Mon Sep 17 00:00:00 2001 From: Finn Plummer <canadienf...@gmail.com> Date: Wed, 9 Jul 2025 15:41:27 +0000 Subject: [PATCH 15/15] self-review: fix typo --- clang/include/clang/Sema/SemaHLSL.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/clang/include/clang/Sema/SemaHLSL.h b/clang/include/clang/Sema/SemaHLSL.h index 42abd29b01ea7..c0da80a70bb82 100644 --- a/clang/include/clang/Sema/SemaHLSL.h +++ b/clang/include/clang/Sema/SemaHLSL.h @@ -153,7 +153,8 @@ class SemaHLSL : public SemaBase { ActOnFinishRootSignatureDecl(SourceLocation Loc, IdentifierInfo *DeclIdent, ArrayRef<hlsl::RootSignatureElement> Elements); - // Returns true when D is invalid and a diagnostic was produced + // Returns true if any RootSignatureElement is invalid and a diagnostic was + // produced bool handleRootSignatureElements(ArrayRef<hlsl::RootSignatureElement> Elements); void handleRootSignatureAttr(Decl *D, const ParsedAttr &AL); _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits