From 9887b09658c210d12efe0984d77adcee9ea99b3f Mon Sep 17 00:00:00 2001 From: Nielsbishere Date: Mon, 14 Jul 2025 22:36:30 +0200 Subject: [PATCH 01/10] Added rewriter option ConsistentBindings (-consistent-bindings) for the IDxcRewriter, this will allow generating register ids for registers that don't fully define them. --- include/dxc/Support/HLSLOptions.h | 1 + include/dxc/Support/HLSLOptions.td | 2 + lib/DxcSupport/HLSLOptions.cpp | 1 + .../clang/tools/libclang/dxcrewriteunused.cpp | 211 ++++++++++++++++++ 4 files changed, 215 insertions(+) diff --git a/include/dxc/Support/HLSLOptions.h b/include/dxc/Support/HLSLOptions.h index 31ca3d1c14..ac1b311e1f 100644 --- a/include/dxc/Support/HLSLOptions.h +++ b/include/dxc/Support/HLSLOptions.h @@ -103,6 +103,7 @@ class DxcDefines { struct RewriterOpts { bool Unchanged = false; // OPT_rw_unchanged + bool ConsistentBindings = false; // OPT_rw_consistent_bindings bool SkipFunctionBody = false; // OPT_rw_skip_function_body bool SkipStatic = false; // OPT_rw_skip_static bool GlobalExternByDefault = false; // OPT_rw_global_extern_by_default diff --git a/include/dxc/Support/HLSLOptions.td b/include/dxc/Support/HLSLOptions.td index 58f6bdfbf3..fb597b8460 100644 --- a/include/dxc/Support/HLSLOptions.td +++ b/include/dxc/Support/HLSLOptions.td @@ -563,6 +563,8 @@ def nologo : Flag<["-", "/"], "nologo">, Group, Flags<[DriverOpt def rw_unchanged : Flag<["-", "/"], "unchanged">, Group, Flags<[RewriteOption]>, HelpText<"Rewrite HLSL, without changes.">; +def rw_consistent_bindings : Flag<["-", "/"], "consistent-bindings">, Group, Flags<[RewriteOption]>, + HelpText<"Generate bindings for registers that aren't fully qualified (to have consistent bindings).">; def rw_skip_function_body : Flag<["-", "/"], "skip-fn-body">, Group, Flags<[RewriteOption]>, HelpText<"Translate function definitions to declarations">; def rw_skip_static : Flag<["-", "/"], "skip-static">, Group, Flags<[RewriteOption]>, diff --git a/lib/DxcSupport/HLSLOptions.cpp b/lib/DxcSupport/HLSLOptions.cpp index eb071eb0a6..638cbf0ad8 100644 --- a/lib/DxcSupport/HLSLOptions.cpp +++ b/lib/DxcSupport/HLSLOptions.cpp @@ -1349,6 +1349,7 @@ int ReadDxcOpts(const OptTable *optionTable, unsigned flagsToInclude, // Rewriter Options if (flagsToInclude & hlsl::options::RewriteOption) { opts.RWOpt.Unchanged = Args.hasFlag(OPT_rw_unchanged, OPT_INVALID, false); + opts.RWOpt.ConsistentBindings = Args.hasFlag(OPT_rw_consistent_bindings, OPT_INVALID, false); opts.RWOpt.SkipFunctionBody = Args.hasFlag(OPT_rw_skip_function_body, OPT_INVALID, false); opts.RWOpt.SkipStatic = diff --git a/tools/clang/tools/libclang/dxcrewriteunused.cpp b/tools/clang/tools/libclang/dxcrewriteunused.cpp index c29854077b..2515f9d7c6 100644 --- a/tools/clang/tools/libclang/dxcrewriteunused.cpp +++ b/tools/clang/tools/libclang/dxcrewriteunused.cpp @@ -12,6 +12,7 @@ #include "clang/AST/ASTConsumer.h" #include "clang/AST/ASTContext.h" #include "clang/AST/RecursiveASTVisitor.h" +#include "clang/AST/HlslTypes.h" #include "clang/Basic/Diagnostic.h" #include "clang/Basic/FileManager.h" #include "clang/Basic/SourceManager.h" @@ -1032,6 +1033,212 @@ static void RemoveStaticDecls(DeclContext &Ctx) { } } +struct ResourceKey { + + uint32_t space; + DXIL::ResourceClass resourceClass; + + bool operator==(const ResourceKey &other) const { + return space == other.space && resourceClass == other.resourceClass; + } +}; + +namespace llvm { +template<> +struct DenseMapInfo { + static inline ResourceKey getEmptyKey() { + return { ~0u, DXIL::ResourceClass::Invalid }; + } + static inline ResourceKey getTombstoneKey() { + return { ~0u - 1, DXIL::ResourceClass::Invalid }; + } + static unsigned getHashValue(const ResourceKey &K) { + return llvm::hash_combine(K.space, uint32_t(K.resourceClass)); + } + static bool isEqual(const ResourceKey &LHS, const ResourceKey &RHS) { + return LHS.space == RHS.space && LHS.resourceClass == RHS.resourceClass; + } +}; +} // namespace llvm + +using RegisterRange = std::pair; //(startReg, count) +using RegisterMap = llvm::DenseMap>; + +//Find gap in register list and fill it + +uint32_t FillNextRegister(llvm::SmallVector &ranges, + uint32_t arraySize) { + + if (ranges.empty()) { + ranges.push_back({ 0, arraySize }); + return 0; + } + + size_t i = 0, j = ranges.size(); + size_t curr = 0; + + for (; i < j; ++i) { + + const RegisterRange& range = ranges[i]; + + if (range.first - curr >= arraySize) { + ranges.insert(ranges.begin() + i, RegisterRange{curr, arraySize}); + return curr; + } + + curr = range.first + range.second; + } + + ranges.emplace_back(RegisterRange{curr, arraySize}); + return curr; +} + +//Insert in the right place (keep sorted) + +void FillRegisterAt(llvm::SmallVector &ranges, + uint32_t registerNr, uint32_t arraySize, + clang::DiagnosticsEngine &diags, const SourceLocation& location) { + + size_t i = 0, j = ranges.size(); + + for (; i < j; ++i) { + + const RegisterRange& range = ranges[i]; + + if (range.first > registerNr) { + + if (registerNr + arraySize > range.first) { + diags.Report(location, diag::err_hlsl_register_semantics_conflicting); + return; + } + + ranges.insert(ranges.begin() + i, RegisterRange{ registerNr, arraySize }); + break; + } + + if (range.first + range.second > registerNr) { + diags.Report(location, diag::err_hlsl_register_semantics_conflicting); + return; + } + } + + if (i == j) + ranges.emplace_back(RegisterRange{registerNr, arraySize}); +} + +static void GenerateConsistentBindings(DeclContext &Ctx, uint32_t autoBindingSpace) { + + clang::DiagnosticsEngine &Diags = + Ctx.getParentASTContext().getDiagnostics(); + + RegisterMap map; + DenseSet> unresolvedRegisters; + + //Fill up map with fully qualified registers to avoid colliding with them later + + for (auto it = Ctx.decls_begin(); it != Ctx.decls_end(); ++it) { + + VarDecl *VD = dyn_cast(*it); + + if (!VD) + continue; + + HLSLResourceAttr *resource = VD->getAttr(); + + if (!resource) + continue; + + uint32_t arraySize = 1; + + if (const ConstantArrayType *arr = dyn_cast(VD->getType())) + arraySize = arr->getSize().getZExtValue(); + + const ArrayRef &UA = VD->getUnusualAnnotations(); + + bool qualified = false; + RegisterAssignment *reg = nullptr; + + for (auto It = UA.begin(), E = UA.end(); It != E; ++It) { + + if ((*It)->getKind() != hlsl::UnusualAnnotation::UA_RegisterAssignment) + continue; + + reg = cast(*It); + + if (!reg->RegisterType) //Unqualified register assignment + break; + + uint32_t space = reg->RegisterSpace.hasValue() + ? reg->RegisterSpace.getValue() + : autoBindingSpace; + + qualified = true; + FillRegisterAt(map[ResourceKey{space, resource->getResClass()}], + reg->RegisterNumber, arraySize, Diags, VD->getLocation()); + break; + } + + if (!qualified) + unresolvedRegisters.insert({VD, reg}); + } + + //Resolve unresolved registers (while avoiding collisions) + + for (const auto& [VD, reg] : unresolvedRegisters) { + + HLSLResourceAttr *resource = VD->getAttr(); + + uint32_t arraySize = 1; + + if (const ConstantArrayType *arr = dyn_cast(VD->getType())) + arraySize = arr->getSize().getZExtValue(); + + char prefix = 't'; + + switch (resource->getResClass()) { + + case DXIL::ResourceClass::Sampler: + prefix = 's'; + break; + + case DXIL::ResourceClass::CBuffer: + prefix = 'b'; + break; + + case DXIL::ResourceClass::UAV: + prefix = 'u'; + break; + } + + uint32_t space = reg ? reg->RegisterSpace.getValue() : autoBindingSpace; + uint32_t registerNr = FillNextRegister(map[ResourceKey{ space, resource->getResClass() }], arraySize); + + if (reg) + { + reg->RegisterType = prefix; + reg->RegisterNumber = registerNr; + reg->setIsValid(true); + } + else + { + hlsl::RegisterAssignment r; //Keep space empty to ensure space overrides still work fine + r.RegisterNumber = registerNr; + r.RegisterType = prefix; + r.setIsValid(true); + + const ArrayRef &UA = + VD->getUnusualAnnotations(); + + SmallVector newVec; + newVec.append(UA.begin(), UA.end()); + newVec.push_back(new (Ctx.getParentASTContext()) + hlsl::RegisterAssignment(r)); + + VD->setUnusualAnnotations(newVec); + } + } +} + static void GlobalVariableAsExternByDefault(DeclContext &Ctx) { for (auto it = Ctx.decls_begin(); it != Ctx.decls_end();) { auto cur = it++; @@ -1065,6 +1272,10 @@ static HRESULT DoSimpleReWrite(DxcLangExtensionsHelper *pHelper, TranslationUnitDecl *tu = astHelper.tu; + if (opts.RWOpt.ConsistentBindings) { + GenerateConsistentBindings(*tu, opts.AutoBindingSpace); + } + if (opts.RWOpt.SkipStatic && opts.RWOpt.SkipFunctionBody) { // Remove static functions and globals. RemoveStaticDecls(*tu); From 845427439c6d8ac51c2b0170472f2c19b697cc47 Mon Sep 17 00:00:00 2001 From: Nielsbishere Date: Tue, 15 Jul 2025 19:42:55 +0200 Subject: [PATCH 02/10] Option -consistent-bindings now properly generates bindings for not fully qualified registers. --- tools/clang/include/clang/AST/HlslTypes.h | 1 + tools/clang/lib/AST/HlslTypes.cpp | 8 +++ .../clang/tools/libclang/dxcrewriteunused.cpp | 51 +++++++++++-------- 3 files changed, 40 insertions(+), 20 deletions(-) diff --git a/tools/clang/include/clang/AST/HlslTypes.h b/tools/clang/include/clang/AST/HlslTypes.h index 43c1effdb8..2caa408b08 100644 --- a/tools/clang/include/clang/AST/HlslTypes.h +++ b/tools/clang/include/clang/AST/HlslTypes.h @@ -497,6 +497,7 @@ bool IsHLSLNumericOrAggregateOfNumericType(clang::QualType type); bool IsHLSLCopyableAnnotatableRecord(clang::QualType QT); bool IsHLSLBuiltinRayAttributeStruct(clang::QualType QT); bool IsHLSLAggregateType(clang::QualType type); +hlsl::DXIL::ResourceClass GetHLSLResourceClass(clang::QualType type); clang::QualType GetHLSLResourceResultType(clang::QualType type); clang::QualType GetHLSLNodeIOResultType(clang::ASTContext &astContext, clang::QualType type); diff --git a/tools/clang/lib/AST/HlslTypes.cpp b/tools/clang/lib/AST/HlslTypes.cpp index 00c18a81a9..8c35e2eb59 100644 --- a/tools/clang/lib/AST/HlslTypes.cpp +++ b/tools/clang/lib/AST/HlslTypes.cpp @@ -524,6 +524,14 @@ bool IsHLSLResourceType(clang::QualType type) { return false; } +hlsl::DXIL::ResourceClass GetHLSLResourceClass(clang::QualType type) { + + if (HLSLResourceAttr* attr = getAttr(type)) + return attr->getResClass(); + + return hlsl::DXIL::ResourceClass::Invalid; +} + bool IsHLSLHitObjectType(QualType type) { return nullptr != getAttr(type); } diff --git a/tools/clang/tools/libclang/dxcrewriteunused.cpp b/tools/clang/tools/libclang/dxcrewriteunused.cpp index 2515f9d7c6..23ee3935c9 100644 --- a/tools/clang/tools/libclang/dxcrewriteunused.cpp +++ b/tools/clang/tools/libclang/dxcrewriteunused.cpp @@ -1132,7 +1132,7 @@ static void GenerateConsistentBindings(DeclContext &Ctx, uint32_t autoBindingSpa Ctx.getParentASTContext().getDiagnostics(); RegisterMap map; - DenseSet> unresolvedRegisters; + llvm::SmallVector, 8> unresolvedRegisters; //Fill up map with fully qualified registers to avoid colliding with them later @@ -1143,15 +1143,18 @@ static void GenerateConsistentBindings(DeclContext &Ctx, uint32_t autoBindingSpa if (!VD) continue; - HLSLResourceAttr *resource = VD->getAttr(); - - if (!resource) - continue; - uint32_t arraySize = 1; + QualType type = VD->getType(); - if (const ConstantArrayType *arr = dyn_cast(VD->getType())) + if (const ConstantArrayType *arr = dyn_cast(VD->getType())) { arraySize = arr->getSize().getZExtValue(); + type = arr->getElementType(); + } + + if (!IsHLSLResourceType(type)) + continue; + + hlsl::DXIL::ResourceClass resClass = GetHLSLResourceClass(type); const ArrayRef &UA = VD->getUnusualAnnotations(); @@ -1173,29 +1176,32 @@ static void GenerateConsistentBindings(DeclContext &Ctx, uint32_t autoBindingSpa : autoBindingSpace; qualified = true; - FillRegisterAt(map[ResourceKey{space, resource->getResClass()}], + FillRegisterAt(map[ResourceKey{space, resClass}], reg->RegisterNumber, arraySize, Diags, VD->getLocation()); break; } if (!qualified) - unresolvedRegisters.insert({VD, reg}); + unresolvedRegisters.emplace_back(std::pair{VD, reg}); } //Resolve unresolved registers (while avoiding collisions) for (const auto& [VD, reg] : unresolvedRegisters) { - HLSLResourceAttr *resource = VD->getAttr(); - uint32_t arraySize = 1; + QualType type = VD->getType(); - if (const ConstantArrayType *arr = dyn_cast(VD->getType())) + if (const ConstantArrayType *arr = dyn_cast(VD->getType())) { arraySize = arr->getSize().getZExtValue(); + type = arr->getElementType(); + } + + hlsl::DXIL::ResourceClass resClass = GetHLSLResourceClass(type); char prefix = 't'; - switch (resource->getResClass()) { + switch (resClass) { case DXIL::ResourceClass::Sampler: prefix = 's'; @@ -1211,7 +1217,8 @@ static void GenerateConsistentBindings(DeclContext &Ctx, uint32_t autoBindingSpa } uint32_t space = reg ? reg->RegisterSpace.getValue() : autoBindingSpace; - uint32_t registerNr = FillNextRegister(map[ResourceKey{ space, resource->getResClass() }], arraySize); + uint32_t registerNr = + FillNextRegister(map[ResourceKey{space, resClass}], arraySize); if (reg) { @@ -1226,15 +1233,19 @@ static void GenerateConsistentBindings(DeclContext &Ctx, uint32_t autoBindingSpa r.RegisterType = prefix; r.setIsValid(true); + llvm::SmallVector annotations; + const ArrayRef &UA = VD->getUnusualAnnotations(); - SmallVector newVec; - newVec.append(UA.begin(), UA.end()); - newVec.push_back(new (Ctx.getParentASTContext()) - hlsl::RegisterAssignment(r)); + for (auto It = UA.begin(), E = UA.end(); It != E; ++It) + annotations.emplace_back(*It); + + annotations.push_back(::new (Ctx.getParentASTContext()) + hlsl::RegisterAssignment(r)); - VD->setUnusualAnnotations(newVec); + VD->setUnusualAnnotations(UnusualAnnotation::CopyToASTContextArray( + Ctx.getParentASTContext(), annotations.data(), annotations.size())); } } } @@ -1294,7 +1305,7 @@ static HRESULT DoSimpleReWrite(DxcLangExtensionsHelper *pHelper, opts.RWOpt.RemoveUnusedFunctions, w); if (FAILED(hr)) return hr; - } else { + } else if(!opts.RWOpt.ConsistentBindings) { o << "// Rewrite unchanged result:\n"; } From f60c594eae64fb5ee3204962e0a3215606d8bfa4 Mon Sep 17 00:00:00 2001 From: Nielsbishere Date: Tue, 15 Jul 2025 21:41:16 +0200 Subject: [PATCH 03/10] Added a test for generating consistent bindings and changed tests to allow passing different rewriter flags. Also added support for cbuffer auto bindings. --- .../HLSL/rewriter/consistent_bindings.hlsl | 48 +++++++ .../consistent_bindings_gold.hlsl | 49 +++++++ .../clang/tools/libclang/dxcrewriteunused.cpp | 124 +++++++++++------- tools/clang/unittests/HLSL/RewriterTest.cpp | 29 ++-- 4 files changed, 191 insertions(+), 59 deletions(-) create mode 100644 tools/clang/test/HLSL/rewriter/consistent_bindings.hlsl create mode 100644 tools/clang/test/HLSL/rewriter/correct_rewrites/consistent_bindings_gold.hlsl diff --git a/tools/clang/test/HLSL/rewriter/consistent_bindings.hlsl b/tools/clang/test/HLSL/rewriter/consistent_bindings.hlsl new file mode 100644 index 0000000000..e12eed0789 --- /dev/null +++ b/tools/clang/test/HLSL/rewriter/consistent_bindings.hlsl @@ -0,0 +1,48 @@ +//UAV + +RWByteAddressBuffer output1; +RWByteAddressBuffer output2; +RWByteAddressBuffer output3 : register(u0); +RWByteAddressBuffer output4 : register(space1); +RWByteAddressBuffer output5 : SEMA; +RWByteAddressBuffer output6; +RWByteAddressBuffer output7 : register(u1); +RWByteAddressBuffer output8[12] : register(u3); +RWByteAddressBuffer output9[12]; +RWByteAddressBuffer output10[33] : register(space1); +RWByteAddressBuffer output11[33] : register(space2); +RWByteAddressBuffer output12[33] : register(u0, space2); + +//SRV + +StructuredBuffer test; +ByteAddressBuffer input13 : SEMA; +ByteAddressBuffer input14; +ByteAddressBuffer input15 : register(t0); +ByteAddressBuffer input16[12] : register(t3); +ByteAddressBuffer input17[2] : register(space1); +ByteAddressBuffer input18[12] : register(t1, space1); +ByteAddressBuffer input19[3] : register(space1); +ByteAddressBuffer input20 : register(space1); + +//Sampler + +SamplerState sampler0; +SamplerState sampler1; +SamplerState sampler2 : register(s0); +SamplerState sampler3 : register(space1); +SamplerState sampler4 : register(s0, space1); + +//CBV + +cbuffer test : register(b0) { float a; }; +cbuffer test2 { float b; }; +cbuffer test3 : register(space1) { float c; }; +cbuffer test4 : register(space1) { float d; }; + +float e; + +[numthreads(16, 16, 1)] +void main(uint id : SV_DispatchThreadID) { + output2.Store(id * 4, 1); //Only use 1 output, but this won't result into output2 receiving wrong bindings +} diff --git a/tools/clang/test/HLSL/rewriter/correct_rewrites/consistent_bindings_gold.hlsl b/tools/clang/test/HLSL/rewriter/correct_rewrites/consistent_bindings_gold.hlsl new file mode 100644 index 0000000000..4c2810e9f4 --- /dev/null +++ b/tools/clang/test/HLSL/rewriter/correct_rewrites/consistent_bindings_gold.hlsl @@ -0,0 +1,49 @@ +RWByteAddressBuffer output1 : register(u2); +RWByteAddressBuffer output2 : register(u15); +RWByteAddressBuffer output3 : register(u0); +RWByteAddressBuffer output4 : register(u0, space1); +RWByteAddressBuffer output5 : SEMA : register(u16); +RWByteAddressBuffer output6 : register(u17); +RWByteAddressBuffer output7 : register(u1); +RWByteAddressBuffer output8[12] : register(u3); +RWByteAddressBuffer output9[12] : register(u18); +RWByteAddressBuffer output10[33] : register(u1, space1); +RWByteAddressBuffer output11[33] : register(u33, space2); +RWByteAddressBuffer output12[33] : register(u0, space2); +StructuredBuffer test : register(t1); +ByteAddressBuffer input13 : SEMA : register(t2); +ByteAddressBuffer input14 : register(t15); +ByteAddressBuffer input15 : register(t0); +ByteAddressBuffer input16[12] : register(t3); +ByteAddressBuffer input17[2] : register(t13, space1); +ByteAddressBuffer input18[12] : register(t1, space1); +ByteAddressBuffer input19[3] : register(t15, space1); +ByteAddressBuffer input20 : register(t0, space1); +SamplerState sampler0 : register(s1); +SamplerState sampler1 : register(s2); +SamplerState sampler2 : register(s0); +SamplerState sampler3 : register(s1, space1); +SamplerState sampler4 : register(s0, space1); +cbuffer test : register(b0) { + const float a; +} +; +cbuffer test2 : register(b1) { + const float b; +} +; +cbuffer test3 : register(b0, space1) { + const float c; +} +; +cbuffer test4 : register(b1, space1) { + const float d; +} +; +const float e; +[numthreads(16, 16, 1)] +void main(uint id : SV_DispatchThreadID) { + output2.Store(id * 4, 1); +} + + diff --git a/tools/clang/tools/libclang/dxcrewriteunused.cpp b/tools/clang/tools/libclang/dxcrewriteunused.cpp index 23ee3935c9..b92d04ebde 100644 --- a/tools/clang/tools/libclang/dxcrewriteunused.cpp +++ b/tools/clang/tools/libclang/dxcrewriteunused.cpp @@ -1064,6 +1064,15 @@ struct DenseMapInfo { using RegisterRange = std::pair; //(startReg, count) using RegisterMap = llvm::DenseMap>; +struct UnresolvedRegister { + hlsl::DXIL::ResourceClass cls; + uint32_t arraySize; + RegisterAssignment *reg; + NamedDecl *ND; +}; + +using UnresolvedRegisters = llvm::SmallVector; + //Find gap in register list and fill it uint32_t FillNextRegister(llvm::SmallVector &ranges, @@ -1126,23 +1135,74 @@ void FillRegisterAt(llvm::SmallVector &ranges, ranges.emplace_back(RegisterRange{registerNr, arraySize}); } +static void RegisterBinding( + NamedDecl *ND, + UnresolvedRegisters& unresolvedRegisters, + RegisterMap& map, + hlsl::DXIL::ResourceClass cls, + uint32_t arraySize, + clang::DiagnosticsEngine &Diags, + uint32_t autoBindingSpace +) { + + const ArrayRef &UA = + ND->getUnusualAnnotations(); + + bool qualified = false; + RegisterAssignment *reg = nullptr; + + for (auto It = UA.begin(), E = UA.end(); It != E; ++It) { + + if ((*It)->getKind() != hlsl::UnusualAnnotation::UA_RegisterAssignment) + continue; + + reg = cast(*It); + + if (!reg->RegisterType) //Unqualified register assignment + break; + + uint32_t space = reg->RegisterSpace.hasValue() + ? reg->RegisterSpace.getValue() + : autoBindingSpace; + + qualified = true; + FillRegisterAt(map[ResourceKey{space, cls }], + reg->RegisterNumber, arraySize, Diags, ND->getLocation()); + break; + } + + if (!qualified) + unresolvedRegisters.emplace_back(UnresolvedRegister{cls, arraySize, reg, ND}); +} + static void GenerateConsistentBindings(DeclContext &Ctx, uint32_t autoBindingSpace) { clang::DiagnosticsEngine &Diags = Ctx.getParentASTContext().getDiagnostics(); RegisterMap map; - llvm::SmallVector, 8> unresolvedRegisters; + UnresolvedRegisters unresolvedRegisters; //Fill up map with fully qualified registers to avoid colliding with them later for (auto it = Ctx.decls_begin(); it != Ctx.decls_end(); ++it) { - VarDecl *VD = dyn_cast(*it); + //CBuffer has special logic, since it's not technically + + if (HLSLBufferDecl *CBuffer = dyn_cast(*it)) { + RegisterBinding(CBuffer, unresolvedRegisters, map, + hlsl::DXIL::ResourceClass::CBuffer, 1, Diags, + autoBindingSpace); + continue; + } + + ValueDecl *VD = dyn_cast(*it); if (!VD) continue; + std::string test = VD->getName(); + uint32_t arraySize = 1; QualType type = VD->getType(); @@ -1154,50 +1214,16 @@ static void GenerateConsistentBindings(DeclContext &Ctx, uint32_t autoBindingSpa if (!IsHLSLResourceType(type)) continue; - hlsl::DXIL::ResourceClass resClass = GetHLSLResourceClass(type); - - const ArrayRef &UA = VD->getUnusualAnnotations(); - - bool qualified = false; - RegisterAssignment *reg = nullptr; - - for (auto It = UA.begin(), E = UA.end(); It != E; ++It) { - - if ((*It)->getKind() != hlsl::UnusualAnnotation::UA_RegisterAssignment) - continue; - - reg = cast(*It); - - if (!reg->RegisterType) //Unqualified register assignment - break; - - uint32_t space = reg->RegisterSpace.hasValue() - ? reg->RegisterSpace.getValue() - : autoBindingSpace; - - qualified = true; - FillRegisterAt(map[ResourceKey{space, resClass}], - reg->RegisterNumber, arraySize, Diags, VD->getLocation()); - break; - } - - if (!qualified) - unresolvedRegisters.emplace_back(std::pair{VD, reg}); + RegisterBinding(VD, unresolvedRegisters, map, GetHLSLResourceClass(type), + arraySize, Diags, autoBindingSpace); } //Resolve unresolved registers (while avoiding collisions) - for (const auto& [VD, reg] : unresolvedRegisters) { + for (const UnresolvedRegister& reg : unresolvedRegisters) { - uint32_t arraySize = 1; - QualType type = VD->getType(); - - if (const ConstantArrayType *arr = dyn_cast(VD->getType())) { - arraySize = arr->getSize().getZExtValue(); - type = arr->getElementType(); - } - - hlsl::DXIL::ResourceClass resClass = GetHLSLResourceClass(type); + uint32_t arraySize = reg.arraySize; + hlsl::DXIL::ResourceClass resClass = reg.cls; char prefix = 't'; @@ -1216,15 +1242,17 @@ static void GenerateConsistentBindings(DeclContext &Ctx, uint32_t autoBindingSpa break; } - uint32_t space = reg ? reg->RegisterSpace.getValue() : autoBindingSpace; + uint32_t space = + reg.reg ? reg.reg->RegisterSpace.getValue() : autoBindingSpace; + uint32_t registerNr = FillNextRegister(map[ResourceKey{space, resClass}], arraySize); - if (reg) + if (reg.reg) { - reg->RegisterType = prefix; - reg->RegisterNumber = registerNr; - reg->setIsValid(true); + reg.reg->RegisterType = prefix; + reg.reg->RegisterNumber = registerNr; + reg.reg->setIsValid(true); } else { @@ -1236,7 +1264,7 @@ static void GenerateConsistentBindings(DeclContext &Ctx, uint32_t autoBindingSpa llvm::SmallVector annotations; const ArrayRef &UA = - VD->getUnusualAnnotations(); + reg.ND->getUnusualAnnotations(); for (auto It = UA.begin(), E = UA.end(); It != E; ++It) annotations.emplace_back(*It); @@ -1244,7 +1272,7 @@ static void GenerateConsistentBindings(DeclContext &Ctx, uint32_t autoBindingSpa annotations.push_back(::new (Ctx.getParentASTContext()) hlsl::RegisterAssignment(r)); - VD->setUnusualAnnotations(UnusualAnnotation::CopyToASTContextArray( + reg.ND->setUnusualAnnotations(UnusualAnnotation::CopyToASTContextArray( Ctx.getParentASTContext(), annotations.data(), annotations.size())); } } diff --git a/tools/clang/unittests/HLSL/RewriterTest.cpp b/tools/clang/unittests/HLSL/RewriterTest.cpp index 613c8561a3..00125275ff 100644 --- a/tools/clang/unittests/HLSL/RewriterTest.cpp +++ b/tools/clang/unittests/HLSL/RewriterTest.cpp @@ -102,6 +102,7 @@ class RewriterTest : public ::testing::Test { TEST_METHOD(RunExtractUniforms) TEST_METHOD(RunGlobalsUsedInMethod) TEST_METHOD(RunRewriterFails) + TEST_METHOD(GenerateConsistentBindings) dxc::DxcDllSupport m_dllSupport; CComPtr m_pIncludeHandler; @@ -126,16 +127,18 @@ class RewriterTest : public ::testing::Test { ppBlob)); } - VerifyResult CheckVerifies(LPCWSTR path, LPCWSTR goldPath) { + VerifyResult CheckVerifies(LPCWSTR path, LPCWSTR goldPath, + const llvm::SmallVector &args = { L"-HV", L"2016" }) { CComPtr pRewriter; VERIFY_SUCCEEDED(CreateRewriter(&pRewriter)); - return CheckVerifies(pRewriter, path, goldPath); + return CheckVerifies(pRewriter, path, goldPath, args); } VerifyResult CheckVerifies(IDxcRewriter *pRewriter, LPCWSTR path, - LPCWSTR goldPath) { + LPCWSTR goldPath, + const llvm::SmallVector &args = { L"-HV", L"2016" }) { CComPtr pRewriteResult; - RewriteCompareGold(path, goldPath, &pRewriteResult, pRewriter); + RewriteCompareGold(path, goldPath, &pRewriteResult, pRewriter, args); VerifyResult toReturn; @@ -165,9 +168,9 @@ class RewriterTest : public ::testing::Test { return S_OK; } - VerifyResult CheckVerifiesHLSL(LPCWSTR name, LPCWSTR goldName) { + VerifyResult CheckVerifiesHLSL(LPCWSTR name, LPCWSTR goldName, const llvm::SmallVector &args = { L"-HV", L"2016" }) { return CheckVerifies(GetPathToHlslDataFile(name).c_str(), - GetPathToHlslDataFile(goldName).c_str()); + GetPathToHlslDataFile(goldName).c_str(), args); } struct FileWithBlob { @@ -210,7 +213,8 @@ class RewriterTest : public ::testing::Test { void RewriteCompareGold(LPCWSTR path, LPCWSTR goldPath, IDxcOperationResult **ppResult, - IDxcRewriter *rewriter) { + IDxcRewriter *rewriter, + const llvm::SmallVector &args = {}) { // Get the source text from a file FileWithBlob source(m_dllSupport, path); @@ -218,13 +222,11 @@ class RewriterTest : public ::testing::Test { DxcDefine myDefines[myDefinesCount] = { {L"myDefine", L"2"}, {L"myDefine3", L"1994"}, {L"myDefine4", nullptr}}; - LPCWSTR args[] = {L"-HV", L"2016"}; - CComPtr rewriter2; VERIFY_SUCCEEDED(rewriter->QueryInterface(&rewriter2)); // Run rewrite unchanged on the source code VERIFY_SUCCEEDED(rewriter2->RewriteWithOptions( - source.BlobEncoding, path, args, _countof(args), myDefines, + source.BlobEncoding, path, (LPCWSTR*) args.data(), (uint32_t) args.size(), myDefines, myDefinesCount, nullptr, ppResult)); // check for compilation errors @@ -329,7 +331,7 @@ TEST_F(RewriterTest, RunArrayLength) { TEST_F(RewriterTest, RunAttributes) { CheckVerifiesHLSL(L"rewriter\\attributes_noerr.hlsl", - L"rewriter\\correct_rewrites\\attributes_gold.hlsl"); + L"rewriter\\correct_rewrites\\attributes_gold.hlsl"); } TEST_F(RewriterTest, RunAnonymousStruct) { @@ -462,6 +464,11 @@ TEST_F(RewriterTest, RunSpirv) { VERIFY_IS_TRUE(strResult.find("namespace vk") == std::string::npos); } +TEST_F(RewriterTest, GenerateConsistentBindings) { + CheckVerifiesHLSL(L"rewriter\\consistent_bindings.hlsl", + L"rewriter\\correct_rewrites\\consistent_bindings_gold.hlsl", { L"-HV", L"2016", L"-consistent-bindings" }); +} + TEST_F(RewriterTest, RunStructMethods) { CheckVerifiesHLSL(L"rewriter\\struct-methods.hlsl", L"rewriter\\correct_rewrites\\struct-methods_gold.hlsl"); From 74283d158717e300116cbf26fb0b943bd4b5c51a Mon Sep 17 00:00:00 2001 From: Nielsbishere Date: Wed, 16 Jul 2025 15:35:53 +0200 Subject: [PATCH 04/10] Applied clang format --- lib/DxcSupport/HLSLOptions.cpp | 3 +- tools/clang/lib/AST/HlslTypes.cpp | 4 +- .../clang/tools/libclang/dxcrewriteunused.cpp | 145 +++++++++--------- tools/clang/unittests/HLSL/RewriterTest.cpp | 25 +-- 4 files changed, 90 insertions(+), 87 deletions(-) diff --git a/lib/DxcSupport/HLSLOptions.cpp b/lib/DxcSupport/HLSLOptions.cpp index 638cbf0ad8..c733b2a633 100644 --- a/lib/DxcSupport/HLSLOptions.cpp +++ b/lib/DxcSupport/HLSLOptions.cpp @@ -1349,7 +1349,8 @@ int ReadDxcOpts(const OptTable *optionTable, unsigned flagsToInclude, // Rewriter Options if (flagsToInclude & hlsl::options::RewriteOption) { opts.RWOpt.Unchanged = Args.hasFlag(OPT_rw_unchanged, OPT_INVALID, false); - opts.RWOpt.ConsistentBindings = Args.hasFlag(OPT_rw_consistent_bindings, OPT_INVALID, false); + opts.RWOpt.ConsistentBindings = + Args.hasFlag(OPT_rw_consistent_bindings, OPT_INVALID, false); opts.RWOpt.SkipFunctionBody = Args.hasFlag(OPT_rw_skip_function_body, OPT_INVALID, false); opts.RWOpt.SkipStatic = diff --git a/tools/clang/lib/AST/HlslTypes.cpp b/tools/clang/lib/AST/HlslTypes.cpp index 8c35e2eb59..9748e13069 100644 --- a/tools/clang/lib/AST/HlslTypes.cpp +++ b/tools/clang/lib/AST/HlslTypes.cpp @@ -526,8 +526,8 @@ bool IsHLSLResourceType(clang::QualType type) { hlsl::DXIL::ResourceClass GetHLSLResourceClass(clang::QualType type) { - if (HLSLResourceAttr* attr = getAttr(type)) - return attr->getResClass(); + if (HLSLResourceAttr *attr = getAttr(type)) + return attr->getResClass(); return hlsl::DXIL::ResourceClass::Invalid; } diff --git a/tools/clang/tools/libclang/dxcrewriteunused.cpp b/tools/clang/tools/libclang/dxcrewriteunused.cpp index b92d04ebde..bf7cf1e702 100644 --- a/tools/clang/tools/libclang/dxcrewriteunused.cpp +++ b/tools/clang/tools/libclang/dxcrewriteunused.cpp @@ -11,8 +11,8 @@ #include "clang/AST/ASTConsumer.h" #include "clang/AST/ASTContext.h" -#include "clang/AST/RecursiveASTVisitor.h" #include "clang/AST/HlslTypes.h" +#include "clang/AST/RecursiveASTVisitor.h" #include "clang/Basic/Diagnostic.h" #include "clang/Basic/FileManager.h" #include "clang/Basic/SourceManager.h" @@ -1044,13 +1044,12 @@ struct ResourceKey { }; namespace llvm { -template<> -struct DenseMapInfo { +template <> struct DenseMapInfo { static inline ResourceKey getEmptyKey() { - return { ~0u, DXIL::ResourceClass::Invalid }; + return {~0u, DXIL::ResourceClass::Invalid}; } static inline ResourceKey getTombstoneKey() { - return { ~0u - 1, DXIL::ResourceClass::Invalid }; + return {~0u - 1, DXIL::ResourceClass::Invalid}; } static unsigned getHashValue(const ResourceKey &K) { return llvm::hash_combine(K.space, uint32_t(K.resourceClass)); @@ -1062,7 +1061,8 @@ struct DenseMapInfo { } // namespace llvm using RegisterRange = std::pair; //(startReg, count) -using RegisterMap = llvm::DenseMap>; +using RegisterMap = + llvm::DenseMap>; struct UnresolvedRegister { hlsl::DXIL::ResourceClass cls; @@ -1073,13 +1073,13 @@ struct UnresolvedRegister { using UnresolvedRegisters = llvm::SmallVector; -//Find gap in register list and fill it +// Find gap in register list and fill it uint32_t FillNextRegister(llvm::SmallVector &ranges, uint32_t arraySize) { - + if (ranges.empty()) { - ranges.push_back({ 0, arraySize }); + ranges.push_back({0, arraySize}); return 0; } @@ -1088,7 +1088,7 @@ uint32_t FillNextRegister(llvm::SmallVector &ranges, for (; i < j; ++i) { - const RegisterRange& range = ranges[i]; + const RegisterRange &range = ranges[i]; if (range.first - curr >= arraySize) { ranges.insert(ranges.begin() + i, RegisterRange{curr, arraySize}); @@ -1102,32 +1102,33 @@ uint32_t FillNextRegister(llvm::SmallVector &ranges, return curr; } -//Insert in the right place (keep sorted) +// Insert in the right place (keep sorted) void FillRegisterAt(llvm::SmallVector &ranges, - uint32_t registerNr, uint32_t arraySize, - clang::DiagnosticsEngine &diags, const SourceLocation& location) { + uint32_t registerNr, uint32_t arraySize, + clang::DiagnosticsEngine &diags, + const SourceLocation &location) { size_t i = 0, j = ranges.size(); for (; i < j; ++i) { - const RegisterRange& range = ranges[i]; + const RegisterRange &range = ranges[i]; if (range.first > registerNr) { - + if (registerNr + arraySize > range.first) { diags.Report(location, diag::err_hlsl_register_semantics_conflicting); return; } - ranges.insert(ranges.begin() + i, RegisterRange{ registerNr, arraySize }); + ranges.insert(ranges.begin() + i, RegisterRange{registerNr, arraySize}); break; } if (range.first + range.second > registerNr) { - diags.Report(location, diag::err_hlsl_register_semantics_conflicting); - return; + diags.Report(location, diag::err_hlsl_register_semantics_conflicting); + return; } } @@ -1135,64 +1136,61 @@ void FillRegisterAt(llvm::SmallVector &ranges, ranges.emplace_back(RegisterRange{registerNr, arraySize}); } -static void RegisterBinding( - NamedDecl *ND, - UnresolvedRegisters& unresolvedRegisters, - RegisterMap& map, - hlsl::DXIL::ResourceClass cls, - uint32_t arraySize, - clang::DiagnosticsEngine &Diags, - uint32_t autoBindingSpace -) { +static void RegisterBinding(NamedDecl *ND, + UnresolvedRegisters &unresolvedRegisters, + RegisterMap &map, hlsl::DXIL::ResourceClass cls, + uint32_t arraySize, clang::DiagnosticsEngine &Diags, + uint32_t autoBindingSpace) { - const ArrayRef &UA = - ND->getUnusualAnnotations(); + const ArrayRef &UA = ND->getUnusualAnnotations(); bool qualified = false; RegisterAssignment *reg = nullptr; for (auto It = UA.begin(), E = UA.end(); It != E; ++It) { - if ((*It)->getKind() != hlsl::UnusualAnnotation::UA_RegisterAssignment) + if ((*It)->getKind() != hlsl::UnusualAnnotation::UA_RegisterAssignment) continue; - reg = cast(*It); + reg = cast(*It); - if (!reg->RegisterType) //Unqualified register assignment - break; + if (!reg->RegisterType) // Unqualified register assignment + break; - uint32_t space = reg->RegisterSpace.hasValue() - ? reg->RegisterSpace.getValue() - : autoBindingSpace; + uint32_t space = reg->RegisterSpace.hasValue() + ? reg->RegisterSpace.getValue() + : autoBindingSpace; - qualified = true; - FillRegisterAt(map[ResourceKey{space, cls }], - reg->RegisterNumber, arraySize, Diags, ND->getLocation()); - break; + qualified = true; + FillRegisterAt(map[ResourceKey{space, cls}], reg->RegisterNumber, arraySize, + Diags, ND->getLocation()); + break; } if (!qualified) - unresolvedRegisters.emplace_back(UnresolvedRegister{cls, arraySize, reg, ND}); + unresolvedRegisters.emplace_back( + UnresolvedRegister{cls, arraySize, reg, ND}); } -static void GenerateConsistentBindings(DeclContext &Ctx, uint32_t autoBindingSpace) { +static void GenerateConsistentBindings(DeclContext &Ctx, + uint32_t autoBindingSpace) { - clang::DiagnosticsEngine &Diags = - Ctx.getParentASTContext().getDiagnostics(); + clang::DiagnosticsEngine &Diags = Ctx.getParentASTContext().getDiagnostics(); RegisterMap map; UnresolvedRegisters unresolvedRegisters; - //Fill up map with fully qualified registers to avoid colliding with them later + // Fill up map with fully qualified registers to avoid colliding with them + // later for (auto it = Ctx.decls_begin(); it != Ctx.decls_end(); ++it) { - //CBuffer has special logic, since it's not technically + // CBuffer has special logic, since it's not technically if (HLSLBufferDecl *CBuffer = dyn_cast(*it)) { RegisterBinding(CBuffer, unresolvedRegisters, map, - hlsl::DXIL::ResourceClass::CBuffer, 1, Diags, - autoBindingSpace); + hlsl::DXIL::ResourceClass::CBuffer, 1, Diags, + autoBindingSpace); continue; } @@ -1206,11 +1204,12 @@ static void GenerateConsistentBindings(DeclContext &Ctx, uint32_t autoBindingSpa uint32_t arraySize = 1; QualType type = VD->getType(); - if (const ConstantArrayType *arr = dyn_cast(VD->getType())) { + if (const ConstantArrayType *arr = + dyn_cast(VD->getType())) { arraySize = arr->getSize().getZExtValue(); type = arr->getElementType(); } - + if (!IsHLSLResourceType(type)) continue; @@ -1218,9 +1217,9 @@ static void GenerateConsistentBindings(DeclContext &Ctx, uint32_t autoBindingSpa arraySize, Diags, autoBindingSpace); } - //Resolve unresolved registers (while avoiding collisions) + // Resolve unresolved registers (while avoiding collisions) - for (const UnresolvedRegister& reg : unresolvedRegisters) { + for (const UnresolvedRegister ® : unresolvedRegisters) { uint32_t arraySize = reg.arraySize; hlsl::DXIL::ResourceClass resClass = reg.cls; @@ -1248,32 +1247,30 @@ static void GenerateConsistentBindings(DeclContext &Ctx, uint32_t autoBindingSpa uint32_t registerNr = FillNextRegister(map[ResourceKey{space, resClass}], arraySize); - if (reg.reg) - { - reg.reg->RegisterType = prefix; - reg.reg->RegisterNumber = registerNr; - reg.reg->setIsValid(true); - } - else - { - hlsl::RegisterAssignment r; //Keep space empty to ensure space overrides still work fine - r.RegisterNumber = registerNr; - r.RegisterType = prefix; - r.setIsValid(true); + if (reg.reg) { + reg.reg->RegisterType = prefix; + reg.reg->RegisterNumber = registerNr; + reg.reg->setIsValid(true); + } else { + hlsl::RegisterAssignment + r; // Keep space empty to ensure space overrides still work fine + r.RegisterNumber = registerNr; + r.RegisterType = prefix; + r.setIsValid(true); - llvm::SmallVector annotations; + llvm::SmallVector annotations; - const ArrayRef &UA = - reg.ND->getUnusualAnnotations(); + const ArrayRef &UA = + reg.ND->getUnusualAnnotations(); - for (auto It = UA.begin(), E = UA.end(); It != E; ++It) - annotations.emplace_back(*It); + for (auto It = UA.begin(), E = UA.end(); It != E; ++It) + annotations.emplace_back(*It); - annotations.push_back(::new (Ctx.getParentASTContext()) - hlsl::RegisterAssignment(r)); + annotations.push_back(::new (Ctx.getParentASTContext()) + hlsl::RegisterAssignment(r)); - reg.ND->setUnusualAnnotations(UnusualAnnotation::CopyToASTContextArray( - Ctx.getParentASTContext(), annotations.data(), annotations.size())); + reg.ND->setUnusualAnnotations(UnusualAnnotation::CopyToASTContextArray( + Ctx.getParentASTContext(), annotations.data(), annotations.size())); } } } @@ -1333,7 +1330,7 @@ static HRESULT DoSimpleReWrite(DxcLangExtensionsHelper *pHelper, opts.RWOpt.RemoveUnusedFunctions, w); if (FAILED(hr)) return hr; - } else if(!opts.RWOpt.ConsistentBindings) { + } else if (!opts.RWOpt.ConsistentBindings) { o << "// Rewrite unchanged result:\n"; } diff --git a/tools/clang/unittests/HLSL/RewriterTest.cpp b/tools/clang/unittests/HLSL/RewriterTest.cpp index 00125275ff..7adfd3f88e 100644 --- a/tools/clang/unittests/HLSL/RewriterTest.cpp +++ b/tools/clang/unittests/HLSL/RewriterTest.cpp @@ -128,15 +128,16 @@ class RewriterTest : public ::testing::Test { } VerifyResult CheckVerifies(LPCWSTR path, LPCWSTR goldPath, - const llvm::SmallVector &args = { L"-HV", L"2016" }) { + const llvm::SmallVector &args = { + L"-HV", L"2016"}) { CComPtr pRewriter; VERIFY_SUCCEEDED(CreateRewriter(&pRewriter)); return CheckVerifies(pRewriter, path, goldPath, args); } - VerifyResult CheckVerifies(IDxcRewriter *pRewriter, LPCWSTR path, - LPCWSTR goldPath, - const llvm::SmallVector &args = { L"-HV", L"2016" }) { + VerifyResult + CheckVerifies(IDxcRewriter *pRewriter, LPCWSTR path, LPCWSTR goldPath, + const llvm::SmallVector &args = {L"-HV", L"2016"}) { CComPtr pRewriteResult; RewriteCompareGold(path, goldPath, &pRewriteResult, pRewriter, args); @@ -168,7 +169,9 @@ class RewriterTest : public ::testing::Test { return S_OK; } - VerifyResult CheckVerifiesHLSL(LPCWSTR name, LPCWSTR goldName, const llvm::SmallVector &args = { L"-HV", L"2016" }) { + VerifyResult CheckVerifiesHLSL(LPCWSTR name, LPCWSTR goldName, + const llvm::SmallVector &args = { + L"-HV", L"2016"}) { return CheckVerifies(GetPathToHlslDataFile(name).c_str(), GetPathToHlslDataFile(goldName).c_str(), args); } @@ -226,8 +229,8 @@ class RewriterTest : public ::testing::Test { VERIFY_SUCCEEDED(rewriter->QueryInterface(&rewriter2)); // Run rewrite unchanged on the source code VERIFY_SUCCEEDED(rewriter2->RewriteWithOptions( - source.BlobEncoding, path, (LPCWSTR*) args.data(), (uint32_t) args.size(), myDefines, - myDefinesCount, nullptr, ppResult)); + source.BlobEncoding, path, (LPCWSTR *)args.data(), + (uint32_t)args.size(), myDefines, myDefinesCount, nullptr, ppResult)); // check for compilation errors HRESULT hrStatus; @@ -331,7 +334,7 @@ TEST_F(RewriterTest, RunArrayLength) { TEST_F(RewriterTest, RunAttributes) { CheckVerifiesHLSL(L"rewriter\\attributes_noerr.hlsl", - L"rewriter\\correct_rewrites\\attributes_gold.hlsl"); + L"rewriter\\correct_rewrites\\attributes_gold.hlsl"); } TEST_F(RewriterTest, RunAnonymousStruct) { @@ -465,8 +468,10 @@ TEST_F(RewriterTest, RunSpirv) { } TEST_F(RewriterTest, GenerateConsistentBindings) { - CheckVerifiesHLSL(L"rewriter\\consistent_bindings.hlsl", - L"rewriter\\correct_rewrites\\consistent_bindings_gold.hlsl", { L"-HV", L"2016", L"-consistent-bindings" }); + CheckVerifiesHLSL( + L"rewriter\\consistent_bindings.hlsl", + L"rewriter\\correct_rewrites\\consistent_bindings_gold.hlsl", + {L"-HV", L"2016", L"-consistent-bindings"}); } TEST_F(RewriterTest, RunStructMethods) { From 6f5689d6b84b5ab5d0877ccc862920f6c3ed07e3 Mon Sep 17 00:00:00 2001 From: Nielsbishere Date: Sun, 3 Aug 2025 19:21:41 +0200 Subject: [PATCH 05/10] Now supporting multi dimensional register arrays for -consistent-bindings --- tools/clang/tools/libclang/dxcrewriteunused.cpp | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/tools/clang/tools/libclang/dxcrewriteunused.cpp b/tools/clang/tools/libclang/dxcrewriteunused.cpp index bf7cf1e702..1c1699196c 100644 --- a/tools/clang/tools/libclang/dxcrewriteunused.cpp +++ b/tools/clang/tools/libclang/dxcrewriteunused.cpp @@ -1204,9 +1204,8 @@ static void GenerateConsistentBindings(DeclContext &Ctx, uint32_t arraySize = 1; QualType type = VD->getType(); - if (const ConstantArrayType *arr = - dyn_cast(VD->getType())) { - arraySize = arr->getSize().getZExtValue(); + while (const ConstantArrayType *arr = dyn_cast(type)) { + arraySize *= arr->getSize().getZExtValue(); type = arr->getElementType(); } From 26a7f54ddd247b5e384719cb15b285b4bdf2c574 Mon Sep 17 00:00:00 2001 From: Nielsbishere Date: Tue, 2 Sep 2025 00:39:13 +0200 Subject: [PATCH 06/10] Removed rewriter option -consistent-bindings and now doing it in the dxil lowering properly, it's similar to -flegacy-resource-reservation but distinct in some key ways: the option only works for reserved registers and we need it to work on any register that has been compiled out. We also don't necessarily need the register to stay at the end of DXIL generation, in fact we'd rather not. Because having the register present can cause unintended performance penalties (for example, I use this reflection to know what transitions to issue) --- include/dxc/DXIL/DxilModule.h | 3 + include/dxc/HLSL/HLModule.h | 2 + include/dxc/Support/HLSLOptions.h | 29 +- include/dxc/Support/HLSLOptions.td | 19 +- lib/DXIL/DxilModule.cpp | 10 + lib/DxcSupport/HLSLOptions.cpp | 20 +- lib/HLSL/DxilCondenseResources.cpp | 6 +- lib/HLSL/DxilGenerationPass.cpp | 1 + lib/HLSL/HLModule.cpp | 18 +- .../include/clang/Frontend/CodeGenOptions.h | 2 + tools/clang/lib/CodeGen/CGHLSLMS.cpp | 2 + .../HLSL/rewriter/consistent_bindings.hlsl | 48 ---- .../clang/tools/dxcompiler/dxcompilerobj.cpp | 2 + .../clang/tools/libclang/dxcrewriteunused.cpp | 247 +----------------- tools/clang/unittests/HLSL/RewriterTest.cpp | 8 - 15 files changed, 93 insertions(+), 324 deletions(-) delete mode 100644 tools/clang/test/HLSL/rewriter/consistent_bindings.hlsl diff --git a/include/dxc/DXIL/DxilModule.h b/include/dxc/DXIL/DxilModule.h index 3f1ba12f86..5a91f0b204 100644 --- a/include/dxc/DXIL/DxilModule.h +++ b/include/dxc/DXIL/DxilModule.h @@ -287,6 +287,8 @@ class DxilModule { // Intermediate options that do not make it to DXIL void SetLegacyResourceReservation(bool legacyResourceReservation); bool GetLegacyResourceReservation() const; + void SetConsistentBindings(bool consistentBindings); + bool GetConsistentBindings() const; void ClearIntermediateOptions(); // Hull and Domain shaders. @@ -346,6 +348,7 @@ class DxilModule { enum IntermediateFlags : uint32_t { LegacyResourceReservation = 1 << 0, + ConsistentBindings = 1 << 1 }; llvm::LLVMContext &m_Ctx; diff --git a/include/dxc/HLSL/HLModule.h b/include/dxc/HLSL/HLModule.h index 653fc967d5..90d0c218cd 100644 --- a/include/dxc/HLSL/HLModule.h +++ b/include/dxc/HLSL/HLModule.h @@ -54,6 +54,7 @@ struct HLOptions { bDisableOptimizations(false), PackingStrategy(0), bUseMinPrecision(false), bDX9CompatMode(false), bFXCCompatMode(false), bLegacyResourceReservation(false), bForceZeroStoreLifetimes(false), + bConsistentBindings(false), unused(0) {} uint32_t GetHLOptionsRaw() const; void SetHLOptionsRaw(uint32_t data); @@ -70,6 +71,7 @@ struct HLOptions { unsigned bLegacyResourceReservation : 1; unsigned bForceZeroStoreLifetimes : 1; unsigned bResMayAlias : 1; + unsigned bConsistentBindings : 1; unsigned unused : 19; }; diff --git a/include/dxc/Support/HLSLOptions.h b/include/dxc/Support/HLSLOptions.h index ac1b311e1f..bfa09f56bd 100644 --- a/include/dxc/Support/HLSLOptions.h +++ b/include/dxc/Support/HLSLOptions.h @@ -102,17 +102,23 @@ class DxcDefines { }; struct RewriterOpts { - bool Unchanged = false; // OPT_rw_unchanged - bool ConsistentBindings = false; // OPT_rw_consistent_bindings - bool SkipFunctionBody = false; // OPT_rw_skip_function_body - bool SkipStatic = false; // OPT_rw_skip_static - bool GlobalExternByDefault = false; // OPT_rw_global_extern_by_default - bool KeepUserMacro = false; // OPT_rw_keep_user_macro - bool ExtractEntryUniforms = false; // OPT_rw_extract_entry_uniforms - bool RemoveUnusedGlobals = false; // OPT_rw_remove_unused_globals - bool RemoveUnusedFunctions = false; // OPT_rw_remove_unused_functions - bool WithLineDirective = false; // OPT_rw_line_directive - bool DeclGlobalCB = false; // OPT_rw_decl_global_cb + bool Unchanged = false; // OPT_rw_unchanged + bool ReflectHLSLBasics = false; // OPT_rw_reflect_hlsl_basics + bool ReflectHLSLFunctions = false; // OPT_rw_reflect_hlsl_functions + bool ReflectHLSLNamespaces = false; // OPT_rw_reflect_hlsl_namespaces + bool ReflectHLSLUserTypes = false; // OPT_rw_reflect_hlsl_user_types + bool ReflectHLSLScopes = false; // OPT_rw_reflect_hlsl_scopes + bool ReflectHLSLVariables = false; // OPT_rw_reflect_hlsl_variables + bool ReflectHLSLDisableSymbols = false; // OPT_rw_reflect_hlsl_disable_symbols + bool SkipFunctionBody = false; // OPT_rw_skip_function_body + bool SkipStatic = false; // OPT_rw_skip_static + bool GlobalExternByDefault = false; // OPT_rw_global_extern_by_default + bool KeepUserMacro = false; // OPT_rw_keep_user_macro + bool ExtractEntryUniforms = false; // OPT_rw_extract_entry_uniforms + bool RemoveUnusedGlobals = false; // OPT_rw_remove_unused_globals + bool RemoveUnusedFunctions = false; // OPT_rw_remove_unused_functions + bool WithLineDirective = false; // OPT_rw_line_directive + bool DeclGlobalCB = false; // OPT_rw_decl_global_cb }; /// Use this class to capture all options. @@ -228,6 +234,7 @@ class DxcOpts { std::string TimeTrace = ""; // OPT_ftime_trace[EQ] unsigned TimeTraceGranularity = 500; // OPT_ftime_trace_granularity_EQ bool VerifyDiagnostics = false; // OPT_verify + bool ConsistentBindings = false; // OPT_consistent_bindings // Optimization pass enables, disables and selects OptimizationToggles diff --git a/include/dxc/Support/HLSLOptions.td b/include/dxc/Support/HLSLOptions.td index fb597b8460..9781ad6b2b 100644 --- a/include/dxc/Support/HLSLOptions.td +++ b/include/dxc/Support/HLSLOptions.td @@ -558,13 +558,28 @@ def getprivate : JoinedOrSeparate<["-", "/"], "getprivate">, Flags<[DriverOption def nologo : Flag<["-", "/"], "nologo">, Group, Flags<[DriverOption, HelpHidden]>, HelpText<"Suppress copyright message">; +def consistent_bindings : Flag<["-", "/"], "consistent-bindings">, Group, Flags<[CoreOption]>, + HelpText<"Generate bindings for registers with no pre-declared binding (consistently regardless of optimization).">; + ////////////////////////////////////////////////////////////////////////////// // Rewriter Options def rw_unchanged : Flag<["-", "/"], "unchanged">, Group, Flags<[RewriteOption]>, HelpText<"Rewrite HLSL, without changes.">; -def rw_consistent_bindings : Flag<["-", "/"], "consistent-bindings">, Group, Flags<[RewriteOption]>, - HelpText<"Generate bindings for registers that aren't fully qualified (to have consistent bindings).">; +def rw_reflect_hlsl_basics : Flag<["-", "/"], "reflect-hlsl-basics">, Group, Flags<[RewriteOption]>, + HelpText<"Reflect HLSL (registers, constants and annotations) and generate consistent bindings.">; +def rw_reflect_hlsl_functions : Flag<["-", "/"], "reflect-hlsl-functions">, Group, Flags<[RewriteOption]>, + HelpText<"Reflect HLSL functions.">; +def rw_reflect_hlsl_namespaces : Flag<["-", "/"], "reflect-hlsl-namespaces">, Group, Flags<[RewriteOption]>, + HelpText<"Reflect HLSL nodes in namespaces rather than only the root namespace.">; +def rw_reflect_hlsl_user_types : Flag<["-", "/"], "reflect-hlsl-user-types">, Group, Flags<[RewriteOption]>, + HelpText<"Reflect HLSL user types (typedef, using, struct, enum).">; +def rw_reflect_hlsl_scopes : Flag<["-", "/"], "reflect-hlsl-scopes">, Group, Flags<[RewriteOption]>, + HelpText<"Reflect HLSL scopes (variables and structs defined in functions, structs and scopes).">; +def rw_reflect_hlsl_variables : Flag<["-", "/"], "reflect-hlsl-variables">, Group, Flags<[RewriteOption]>, + HelpText<"Reflect HLSL static variables.">; +def rw_reflect_hlsl_disable_symbols : Flag<["-", "/"], "reflect-hlsl-disable-symbols">, Group, Flags<[RewriteOption]>, + HelpText<"Reflect HLSL disable symbols.">; def rw_skip_function_body : Flag<["-", "/"], "skip-fn-body">, Group, Flags<[RewriteOption]>, HelpText<"Translate function definitions to declarations">; def rw_skip_static : Flag<["-", "/"], "skip-static">, Group, Flags<[RewriteOption]>, diff --git a/lib/DXIL/DxilModule.cpp b/lib/DXIL/DxilModule.cpp index f4abdd15aa..d55923d964 100644 --- a/lib/DXIL/DxilModule.cpp +++ b/lib/DXIL/DxilModule.cpp @@ -586,6 +586,16 @@ bool DxilModule::GetLegacyResourceReservation() const { return (m_IntermediateFlags & LegacyResourceReservation) != 0; } +void DxilModule::SetConsistentBindings(bool consistentBindings) { + m_IntermediateFlags &= ~ConsistentBindings; + if (consistentBindings) + m_IntermediateFlags |= ConsistentBindings; +} + +bool DxilModule::GetConsistentBindings() const { + return (m_IntermediateFlags & ConsistentBindings) != 0; +} + void DxilModule::ClearIntermediateOptions() { m_IntermediateFlags = 0; } unsigned DxilModule::GetInputControlPointCount() const { diff --git a/lib/DxcSupport/HLSLOptions.cpp b/lib/DxcSupport/HLSLOptions.cpp index c733b2a633..b6d38edd8a 100644 --- a/lib/DxcSupport/HLSLOptions.cpp +++ b/lib/DxcSupport/HLSLOptions.cpp @@ -871,6 +871,10 @@ int ReadDxcOpts(const OptTable *optionTable, unsigned flagsToInclude, opts.TimeReport = Args.hasFlag(OPT_ftime_report, OPT_INVALID, false); opts.TimeTrace = Args.hasFlag(OPT_ftime_trace, OPT_INVALID, false) ? "-" : ""; opts.VerifyDiagnostics = Args.hasFlag(OPT_verify, OPT_INVALID, false); + + opts.ConsistentBindings = + Args.hasFlag(OPT_consistent_bindings, OPT_INVALID, false); + if (Args.hasArg(OPT_ftime_trace_EQ)) opts.TimeTrace = Args.getLastArgValue(OPT_ftime_trace_EQ); if (Arg *A = Args.getLastArg(OPT_ftime_trace_granularity_EQ)) { @@ -1349,8 +1353,20 @@ int ReadDxcOpts(const OptTable *optionTable, unsigned flagsToInclude, // Rewriter Options if (flagsToInclude & hlsl::options::RewriteOption) { opts.RWOpt.Unchanged = Args.hasFlag(OPT_rw_unchanged, OPT_INVALID, false); - opts.RWOpt.ConsistentBindings = - Args.hasFlag(OPT_rw_consistent_bindings, OPT_INVALID, false); + opts.RWOpt.ReflectHLSLBasics = + Args.hasFlag(OPT_rw_reflect_hlsl_basics, OPT_INVALID, false); + opts.RWOpt.ReflectHLSLFunctions = + Args.hasFlag(OPT_rw_reflect_hlsl_functions, OPT_INVALID, false); + opts.RWOpt.ReflectHLSLNamespaces = + Args.hasFlag(OPT_rw_reflect_hlsl_namespaces, OPT_INVALID, false); + opts.RWOpt.ReflectHLSLUserTypes = + Args.hasFlag(OPT_rw_reflect_hlsl_user_types, OPT_INVALID, false); + opts.RWOpt.ReflectHLSLScopes = + Args.hasFlag(OPT_rw_reflect_hlsl_scopes, OPT_INVALID, false); + opts.RWOpt.ReflectHLSLVariables = + Args.hasFlag(OPT_rw_reflect_hlsl_variables, OPT_INVALID, false); + opts.RWOpt.ReflectHLSLDisableSymbols = + Args.hasFlag(OPT_rw_reflect_hlsl_disable_symbols, OPT_INVALID, false); opts.RWOpt.SkipFunctionBody = Args.hasFlag(OPT_rw_skip_function_body, OPT_INVALID, false); opts.RWOpt.SkipStatic = diff --git a/lib/HLSL/DxilCondenseResources.cpp b/lib/HLSL/DxilCondenseResources.cpp index 09dd9cea64..18dd741cc8 100644 --- a/lib/HLSL/DxilCondenseResources.cpp +++ b/lib/HLSL/DxilCondenseResources.cpp @@ -550,7 +550,8 @@ class DxilLowerCreateHandleForLib : public ModulePass { ResourceRegisterAllocator.GatherReservedRegisters(DM); // Remove unused resources. - DM.RemoveResourcesWithUnusedSymbols(); + if (!DM.GetConsistentBindings()) + DM.RemoveResourcesWithUnusedSymbols(); unsigned newResources = DM.GetCBuffers().size() + DM.GetUAVs().size() + DM.GetSRVs().size() + DM.GetSamplers().size(); @@ -571,6 +572,9 @@ class DxilLowerCreateHandleForLib : public ModulePass { bChanged |= ResourceRegisterAllocator.AllocateRegisters(DM); + if (DM.GetConsistentBindings()) + DM.RemoveResourcesWithUnusedSymbols(); + // Fill in top-level CBuffer variable usage bit UpdateCBufferUsage(); diff --git a/lib/HLSL/DxilGenerationPass.cpp b/lib/HLSL/DxilGenerationPass.cpp index c3a6ad7dfc..1686482518 100644 --- a/lib/HLSL/DxilGenerationPass.cpp +++ b/lib/HLSL/DxilGenerationPass.cpp @@ -155,6 +155,7 @@ void InitDxilModuleFromHLModule(HLModule &H, DxilModule &M, bool HasDebugInfo) { // bool m_bDisableOptimizations; M.SetDisableOptimization(H.GetHLOptions().bDisableOptimizations); M.SetLegacyResourceReservation(H.GetHLOptions().bLegacyResourceReservation); + M.SetConsistentBindings(H.GetHLOptions().bConsistentBindings); // bool m_bDisableMathRefactoring; // bool m_bEnableDoublePrecision; // bool m_bEnableDoubleExtensions; diff --git a/lib/HLSL/HLModule.cpp b/lib/HLSL/HLModule.cpp index bab6e23a30..ce6241a3b6 100644 --- a/lib/HLSL/HLModule.cpp +++ b/lib/HLSL/HLModule.cpp @@ -231,17 +231,18 @@ void HLModule::RemoveFunction(llvm::Function *F) { namespace { template bool RemoveResource(std::vector> &vec, - GlobalVariable *pVariable, bool keepAllocated) { + GlobalVariable *pVariable, bool keepAllocated, bool dontUpdateIds) { for (auto p = vec.begin(), e = vec.end(); p != e; ++p) { if ((*p)->GetGlobalSymbol() != pVariable) continue; - if (keepAllocated && (*p)->IsAllocated()) { + if ((keepAllocated && (*p)->IsAllocated()) || dontUpdateIds) { // Keep the resource, but it has no more symbol. (*p)->SetGlobalSymbol(UndefValue::get(pVariable->getType())); } else { // Erase the resource alltogether and update IDs of subsequent ones p = vec.erase(p); + for (e = vec.end(); p != e; ++p) { unsigned ID = (*p)->GetID() - 1; (*p)->SetID(ID); @@ -262,16 +263,21 @@ void HLModule::RemoveGlobal(llvm::GlobalVariable *GV) { // register range from being allocated to other resources. bool keepAllocated = GetHLOptions().bLegacyResourceReservation; + // Consistent bindings are different than -flegacy-resource-reservation; + // We need the IDs to stay the same, but it's fine to remove unused registers. + // It's actually wanted, because that allows us to know what registers are optimized out. + bool consistentBindings = GetHLOptions().bConsistentBindings; + // This could be considerably faster - check variable type to see which // resource type this is rather than scanning all lists, and look for // usage and removal patterns. - if (RemoveResource(m_CBuffers, GV, keepAllocated)) + if (RemoveResource(m_CBuffers, GV, keepAllocated, consistentBindings)) return; - if (RemoveResource(m_SRVs, GV, keepAllocated)) + if (RemoveResource(m_SRVs, GV, keepAllocated, consistentBindings)) return; - if (RemoveResource(m_UAVs, GV, keepAllocated)) + if (RemoveResource(m_UAVs, GV, keepAllocated, consistentBindings)) return; - if (RemoveResource(m_Samplers, GV, keepAllocated)) + if (RemoveResource(m_Samplers, GV, keepAllocated, consistentBindings)) return; // TODO: do m_TGSMVariables and m_StreamOutputs need maintenance? } diff --git a/tools/clang/include/clang/Frontend/CodeGenOptions.h b/tools/clang/include/clang/Frontend/CodeGenOptions.h index 859cba53da..2fe3d418b9 100644 --- a/tools/clang/include/clang/Frontend/CodeGenOptions.h +++ b/tools/clang/include/clang/Frontend/CodeGenOptions.h @@ -187,6 +187,8 @@ class CodeGenOptions : public CodeGenOptionsBase { bool HLSLOnlyWarnOnUnrollFail = false; /// Whether use legacy resource reservation. bool HLSLLegacyResourceReservation = false; + /// Whether to keep bindings consistent even if optimized out. + bool HLSLConsistentBindings = false; /// Set [branch] on every if. bool HLSLPreferControlFlow = false; /// Set [flatten] on every if. diff --git a/tools/clang/lib/CodeGen/CGHLSLMS.cpp b/tools/clang/lib/CodeGen/CGHLSLMS.cpp index b5add521a6..92964ba233 100644 --- a/tools/clang/lib/CodeGen/CGHLSLMS.cpp +++ b/tools/clang/lib/CodeGen/CGHLSLMS.cpp @@ -400,6 +400,8 @@ CGMSHLSLRuntime::CGMSHLSLRuntime(CodeGenModule &CGM) opts.PackingStrategy = CGM.getCodeGenOpts().HLSLSignaturePackingStrategy; opts.bLegacyResourceReservation = CGM.getCodeGenOpts().HLSLLegacyResourceReservation; + opts.bConsistentBindings = + CGM.getCodeGenOpts().HLSLConsistentBindings; opts.bForceZeroStoreLifetimes = CGM.getCodeGenOpts().HLSLForceZeroStoreLifetimes; diff --git a/tools/clang/test/HLSL/rewriter/consistent_bindings.hlsl b/tools/clang/test/HLSL/rewriter/consistent_bindings.hlsl deleted file mode 100644 index e12eed0789..0000000000 --- a/tools/clang/test/HLSL/rewriter/consistent_bindings.hlsl +++ /dev/null @@ -1,48 +0,0 @@ -//UAV - -RWByteAddressBuffer output1; -RWByteAddressBuffer output2; -RWByteAddressBuffer output3 : register(u0); -RWByteAddressBuffer output4 : register(space1); -RWByteAddressBuffer output5 : SEMA; -RWByteAddressBuffer output6; -RWByteAddressBuffer output7 : register(u1); -RWByteAddressBuffer output8[12] : register(u3); -RWByteAddressBuffer output9[12]; -RWByteAddressBuffer output10[33] : register(space1); -RWByteAddressBuffer output11[33] : register(space2); -RWByteAddressBuffer output12[33] : register(u0, space2); - -//SRV - -StructuredBuffer test; -ByteAddressBuffer input13 : SEMA; -ByteAddressBuffer input14; -ByteAddressBuffer input15 : register(t0); -ByteAddressBuffer input16[12] : register(t3); -ByteAddressBuffer input17[2] : register(space1); -ByteAddressBuffer input18[12] : register(t1, space1); -ByteAddressBuffer input19[3] : register(space1); -ByteAddressBuffer input20 : register(space1); - -//Sampler - -SamplerState sampler0; -SamplerState sampler1; -SamplerState sampler2 : register(s0); -SamplerState sampler3 : register(space1); -SamplerState sampler4 : register(s0, space1); - -//CBV - -cbuffer test : register(b0) { float a; }; -cbuffer test2 { float b; }; -cbuffer test3 : register(space1) { float c; }; -cbuffer test4 : register(space1) { float d; }; - -float e; - -[numthreads(16, 16, 1)] -void main(uint id : SV_DispatchThreadID) { - output2.Store(id * 4, 1); //Only use 1 output, but this won't result into output2 receiving wrong bindings -} diff --git a/tools/clang/tools/dxcompiler/dxcompilerobj.cpp b/tools/clang/tools/dxcompiler/dxcompilerobj.cpp index 84b568df9c..9989aa4150 100644 --- a/tools/clang/tools/dxcompiler/dxcompilerobj.cpp +++ b/tools/clang/tools/dxcompiler/dxcompilerobj.cpp @@ -1602,6 +1602,8 @@ class DxcCompiler : public IDxcCompiler3, compiler.getCodeGenOpts().HLSLAvoidControlFlow = Opts.AvoidFlowControl; compiler.getCodeGenOpts().HLSLLegacyResourceReservation = Opts.LegacyResourceReservation; + compiler.getCodeGenOpts().HLSLConsistentBindings = + Opts.ConsistentBindings; compiler.getCodeGenOpts().HLSLDefines = defines; compiler.getCodeGenOpts().HLSLPreciseOutputs = Opts.PreciseOutputs; compiler.getCodeGenOpts().MainFileName = pMainFile; diff --git a/tools/clang/tools/libclang/dxcrewriteunused.cpp b/tools/clang/tools/libclang/dxcrewriteunused.cpp index 1c1699196c..23496242a4 100644 --- a/tools/clang/tools/libclang/dxcrewriteunused.cpp +++ b/tools/clang/tools/libclang/dxcrewriteunused.cpp @@ -1033,247 +1033,6 @@ static void RemoveStaticDecls(DeclContext &Ctx) { } } -struct ResourceKey { - - uint32_t space; - DXIL::ResourceClass resourceClass; - - bool operator==(const ResourceKey &other) const { - return space == other.space && resourceClass == other.resourceClass; - } -}; - -namespace llvm { -template <> struct DenseMapInfo { - static inline ResourceKey getEmptyKey() { - return {~0u, DXIL::ResourceClass::Invalid}; - } - static inline ResourceKey getTombstoneKey() { - return {~0u - 1, DXIL::ResourceClass::Invalid}; - } - static unsigned getHashValue(const ResourceKey &K) { - return llvm::hash_combine(K.space, uint32_t(K.resourceClass)); - } - static bool isEqual(const ResourceKey &LHS, const ResourceKey &RHS) { - return LHS.space == RHS.space && LHS.resourceClass == RHS.resourceClass; - } -}; -} // namespace llvm - -using RegisterRange = std::pair; //(startReg, count) -using RegisterMap = - llvm::DenseMap>; - -struct UnresolvedRegister { - hlsl::DXIL::ResourceClass cls; - uint32_t arraySize; - RegisterAssignment *reg; - NamedDecl *ND; -}; - -using UnresolvedRegisters = llvm::SmallVector; - -// Find gap in register list and fill it - -uint32_t FillNextRegister(llvm::SmallVector &ranges, - uint32_t arraySize) { - - if (ranges.empty()) { - ranges.push_back({0, arraySize}); - return 0; - } - - size_t i = 0, j = ranges.size(); - size_t curr = 0; - - for (; i < j; ++i) { - - const RegisterRange &range = ranges[i]; - - if (range.first - curr >= arraySize) { - ranges.insert(ranges.begin() + i, RegisterRange{curr, arraySize}); - return curr; - } - - curr = range.first + range.second; - } - - ranges.emplace_back(RegisterRange{curr, arraySize}); - return curr; -} - -// Insert in the right place (keep sorted) - -void FillRegisterAt(llvm::SmallVector &ranges, - uint32_t registerNr, uint32_t arraySize, - clang::DiagnosticsEngine &diags, - const SourceLocation &location) { - - size_t i = 0, j = ranges.size(); - - for (; i < j; ++i) { - - const RegisterRange &range = ranges[i]; - - if (range.first > registerNr) { - - if (registerNr + arraySize > range.first) { - diags.Report(location, diag::err_hlsl_register_semantics_conflicting); - return; - } - - ranges.insert(ranges.begin() + i, RegisterRange{registerNr, arraySize}); - break; - } - - if (range.first + range.second > registerNr) { - diags.Report(location, diag::err_hlsl_register_semantics_conflicting); - return; - } - } - - if (i == j) - ranges.emplace_back(RegisterRange{registerNr, arraySize}); -} - -static void RegisterBinding(NamedDecl *ND, - UnresolvedRegisters &unresolvedRegisters, - RegisterMap &map, hlsl::DXIL::ResourceClass cls, - uint32_t arraySize, clang::DiagnosticsEngine &Diags, - uint32_t autoBindingSpace) { - - const ArrayRef &UA = ND->getUnusualAnnotations(); - - bool qualified = false; - RegisterAssignment *reg = nullptr; - - for (auto It = UA.begin(), E = UA.end(); It != E; ++It) { - - if ((*It)->getKind() != hlsl::UnusualAnnotation::UA_RegisterAssignment) - continue; - - reg = cast(*It); - - if (!reg->RegisterType) // Unqualified register assignment - break; - - uint32_t space = reg->RegisterSpace.hasValue() - ? reg->RegisterSpace.getValue() - : autoBindingSpace; - - qualified = true; - FillRegisterAt(map[ResourceKey{space, cls}], reg->RegisterNumber, arraySize, - Diags, ND->getLocation()); - break; - } - - if (!qualified) - unresolvedRegisters.emplace_back( - UnresolvedRegister{cls, arraySize, reg, ND}); -} - -static void GenerateConsistentBindings(DeclContext &Ctx, - uint32_t autoBindingSpace) { - - clang::DiagnosticsEngine &Diags = Ctx.getParentASTContext().getDiagnostics(); - - RegisterMap map; - UnresolvedRegisters unresolvedRegisters; - - // Fill up map with fully qualified registers to avoid colliding with them - // later - - for (auto it = Ctx.decls_begin(); it != Ctx.decls_end(); ++it) { - - // CBuffer has special logic, since it's not technically - - if (HLSLBufferDecl *CBuffer = dyn_cast(*it)) { - RegisterBinding(CBuffer, unresolvedRegisters, map, - hlsl::DXIL::ResourceClass::CBuffer, 1, Diags, - autoBindingSpace); - continue; - } - - ValueDecl *VD = dyn_cast(*it); - - if (!VD) - continue; - - std::string test = VD->getName(); - - uint32_t arraySize = 1; - QualType type = VD->getType(); - - while (const ConstantArrayType *arr = dyn_cast(type)) { - arraySize *= arr->getSize().getZExtValue(); - type = arr->getElementType(); - } - - if (!IsHLSLResourceType(type)) - continue; - - RegisterBinding(VD, unresolvedRegisters, map, GetHLSLResourceClass(type), - arraySize, Diags, autoBindingSpace); - } - - // Resolve unresolved registers (while avoiding collisions) - - for (const UnresolvedRegister ® : unresolvedRegisters) { - - uint32_t arraySize = reg.arraySize; - hlsl::DXIL::ResourceClass resClass = reg.cls; - - char prefix = 't'; - - switch (resClass) { - - case DXIL::ResourceClass::Sampler: - prefix = 's'; - break; - - case DXIL::ResourceClass::CBuffer: - prefix = 'b'; - break; - - case DXIL::ResourceClass::UAV: - prefix = 'u'; - break; - } - - uint32_t space = - reg.reg ? reg.reg->RegisterSpace.getValue() : autoBindingSpace; - - uint32_t registerNr = - FillNextRegister(map[ResourceKey{space, resClass}], arraySize); - - if (reg.reg) { - reg.reg->RegisterType = prefix; - reg.reg->RegisterNumber = registerNr; - reg.reg->setIsValid(true); - } else { - hlsl::RegisterAssignment - r; // Keep space empty to ensure space overrides still work fine - r.RegisterNumber = registerNr; - r.RegisterType = prefix; - r.setIsValid(true); - - llvm::SmallVector annotations; - - const ArrayRef &UA = - reg.ND->getUnusualAnnotations(); - - for (auto It = UA.begin(), E = UA.end(); It != E; ++It) - annotations.emplace_back(*It); - - annotations.push_back(::new (Ctx.getParentASTContext()) - hlsl::RegisterAssignment(r)); - - reg.ND->setUnusualAnnotations(UnusualAnnotation::CopyToASTContextArray( - Ctx.getParentASTContext(), annotations.data(), annotations.size())); - } - } -} - static void GlobalVariableAsExternByDefault(DeclContext &Ctx) { for (auto it = Ctx.decls_begin(); it != Ctx.decls_end();) { auto cur = it++; @@ -1307,10 +1066,6 @@ static HRESULT DoSimpleReWrite(DxcLangExtensionsHelper *pHelper, TranslationUnitDecl *tu = astHelper.tu; - if (opts.RWOpt.ConsistentBindings) { - GenerateConsistentBindings(*tu, opts.AutoBindingSpace); - } - if (opts.RWOpt.SkipStatic && opts.RWOpt.SkipFunctionBody) { // Remove static functions and globals. RemoveStaticDecls(*tu); @@ -1329,7 +1084,7 @@ static HRESULT DoSimpleReWrite(DxcLangExtensionsHelper *pHelper, opts.RWOpt.RemoveUnusedFunctions, w); if (FAILED(hr)) return hr; - } else if (!opts.RWOpt.ConsistentBindings) { + o << "// Rewrite unchanged result:\n"; } diff --git a/tools/clang/unittests/HLSL/RewriterTest.cpp b/tools/clang/unittests/HLSL/RewriterTest.cpp index 7adfd3f88e..1130772ff0 100644 --- a/tools/clang/unittests/HLSL/RewriterTest.cpp +++ b/tools/clang/unittests/HLSL/RewriterTest.cpp @@ -102,7 +102,6 @@ class RewriterTest : public ::testing::Test { TEST_METHOD(RunExtractUniforms) TEST_METHOD(RunGlobalsUsedInMethod) TEST_METHOD(RunRewriterFails) - TEST_METHOD(GenerateConsistentBindings) dxc::DxcDllSupport m_dllSupport; CComPtr m_pIncludeHandler; @@ -467,13 +466,6 @@ TEST_F(RewriterTest, RunSpirv) { VERIFY_IS_TRUE(strResult.find("namespace vk") == std::string::npos); } -TEST_F(RewriterTest, GenerateConsistentBindings) { - CheckVerifiesHLSL( - L"rewriter\\consistent_bindings.hlsl", - L"rewriter\\correct_rewrites\\consistent_bindings_gold.hlsl", - {L"-HV", L"2016", L"-consistent-bindings"}); -} - TEST_F(RewriterTest, RunStructMethods) { CheckVerifiesHLSL(L"rewriter\\struct-methods.hlsl", L"rewriter\\correct_rewrites\\struct-methods_gold.hlsl"); From b63cdd2479959c987c420c2b0d73c835bb28232f Mon Sep 17 00:00:00 2001 From: Nielsbishere Date: Tue, 2 Sep 2025 00:56:19 +0200 Subject: [PATCH 07/10] Reset some incorrect merges --- include/dxc/Support/HLSLOptions.h | 7 --- include/dxc/Support/HLSLOptions.td | 14 ------ lib/DxcSupport/HLSLOptions.cpp | 14 ------ tools/clang/include/clang/AST/HlslTypes.h | 1 - tools/clang/lib/AST/HlslTypes.cpp | 8 --- .../consistent_bindings_gold.hlsl | 49 ------------------- .../clang/tools/libclang/dxcrewriteunused.cpp | 2 +- tools/clang/unittests/HLSL/RewriterTest.cpp | 28 +++++------ 8 files changed, 13 insertions(+), 110 deletions(-) delete mode 100644 tools/clang/test/HLSL/rewriter/correct_rewrites/consistent_bindings_gold.hlsl diff --git a/include/dxc/Support/HLSLOptions.h b/include/dxc/Support/HLSLOptions.h index bfa09f56bd..10a41489c7 100644 --- a/include/dxc/Support/HLSLOptions.h +++ b/include/dxc/Support/HLSLOptions.h @@ -103,13 +103,6 @@ class DxcDefines { struct RewriterOpts { bool Unchanged = false; // OPT_rw_unchanged - bool ReflectHLSLBasics = false; // OPT_rw_reflect_hlsl_basics - bool ReflectHLSLFunctions = false; // OPT_rw_reflect_hlsl_functions - bool ReflectHLSLNamespaces = false; // OPT_rw_reflect_hlsl_namespaces - bool ReflectHLSLUserTypes = false; // OPT_rw_reflect_hlsl_user_types - bool ReflectHLSLScopes = false; // OPT_rw_reflect_hlsl_scopes - bool ReflectHLSLVariables = false; // OPT_rw_reflect_hlsl_variables - bool ReflectHLSLDisableSymbols = false; // OPT_rw_reflect_hlsl_disable_symbols bool SkipFunctionBody = false; // OPT_rw_skip_function_body bool SkipStatic = false; // OPT_rw_skip_static bool GlobalExternByDefault = false; // OPT_rw_global_extern_by_default diff --git a/include/dxc/Support/HLSLOptions.td b/include/dxc/Support/HLSLOptions.td index 9781ad6b2b..1b0ac58c7e 100644 --- a/include/dxc/Support/HLSLOptions.td +++ b/include/dxc/Support/HLSLOptions.td @@ -566,20 +566,6 @@ def consistent_bindings : Flag<["-", "/"], "consistent-bindings">, Group, Group, Flags<[RewriteOption]>, HelpText<"Rewrite HLSL, without changes.">; -def rw_reflect_hlsl_basics : Flag<["-", "/"], "reflect-hlsl-basics">, Group, Flags<[RewriteOption]>, - HelpText<"Reflect HLSL (registers, constants and annotations) and generate consistent bindings.">; -def rw_reflect_hlsl_functions : Flag<["-", "/"], "reflect-hlsl-functions">, Group, Flags<[RewriteOption]>, - HelpText<"Reflect HLSL functions.">; -def rw_reflect_hlsl_namespaces : Flag<["-", "/"], "reflect-hlsl-namespaces">, Group, Flags<[RewriteOption]>, - HelpText<"Reflect HLSL nodes in namespaces rather than only the root namespace.">; -def rw_reflect_hlsl_user_types : Flag<["-", "/"], "reflect-hlsl-user-types">, Group, Flags<[RewriteOption]>, - HelpText<"Reflect HLSL user types (typedef, using, struct, enum).">; -def rw_reflect_hlsl_scopes : Flag<["-", "/"], "reflect-hlsl-scopes">, Group, Flags<[RewriteOption]>, - HelpText<"Reflect HLSL scopes (variables and structs defined in functions, structs and scopes).">; -def rw_reflect_hlsl_variables : Flag<["-", "/"], "reflect-hlsl-variables">, Group, Flags<[RewriteOption]>, - HelpText<"Reflect HLSL static variables.">; -def rw_reflect_hlsl_disable_symbols : Flag<["-", "/"], "reflect-hlsl-disable-symbols">, Group, Flags<[RewriteOption]>, - HelpText<"Reflect HLSL disable symbols.">; def rw_skip_function_body : Flag<["-", "/"], "skip-fn-body">, Group, Flags<[RewriteOption]>, HelpText<"Translate function definitions to declarations">; def rw_skip_static : Flag<["-", "/"], "skip-static">, Group, Flags<[RewriteOption]>, diff --git a/lib/DxcSupport/HLSLOptions.cpp b/lib/DxcSupport/HLSLOptions.cpp index b6d38edd8a..77f0e4ce4a 100644 --- a/lib/DxcSupport/HLSLOptions.cpp +++ b/lib/DxcSupport/HLSLOptions.cpp @@ -1353,20 +1353,6 @@ int ReadDxcOpts(const OptTable *optionTable, unsigned flagsToInclude, // Rewriter Options if (flagsToInclude & hlsl::options::RewriteOption) { opts.RWOpt.Unchanged = Args.hasFlag(OPT_rw_unchanged, OPT_INVALID, false); - opts.RWOpt.ReflectHLSLBasics = - Args.hasFlag(OPT_rw_reflect_hlsl_basics, OPT_INVALID, false); - opts.RWOpt.ReflectHLSLFunctions = - Args.hasFlag(OPT_rw_reflect_hlsl_functions, OPT_INVALID, false); - opts.RWOpt.ReflectHLSLNamespaces = - Args.hasFlag(OPT_rw_reflect_hlsl_namespaces, OPT_INVALID, false); - opts.RWOpt.ReflectHLSLUserTypes = - Args.hasFlag(OPT_rw_reflect_hlsl_user_types, OPT_INVALID, false); - opts.RWOpt.ReflectHLSLScopes = - Args.hasFlag(OPT_rw_reflect_hlsl_scopes, OPT_INVALID, false); - opts.RWOpt.ReflectHLSLVariables = - Args.hasFlag(OPT_rw_reflect_hlsl_variables, OPT_INVALID, false); - opts.RWOpt.ReflectHLSLDisableSymbols = - Args.hasFlag(OPT_rw_reflect_hlsl_disable_symbols, OPT_INVALID, false); opts.RWOpt.SkipFunctionBody = Args.hasFlag(OPT_rw_skip_function_body, OPT_INVALID, false); opts.RWOpt.SkipStatic = diff --git a/tools/clang/include/clang/AST/HlslTypes.h b/tools/clang/include/clang/AST/HlslTypes.h index 2caa408b08..43c1effdb8 100644 --- a/tools/clang/include/clang/AST/HlslTypes.h +++ b/tools/clang/include/clang/AST/HlslTypes.h @@ -497,7 +497,6 @@ bool IsHLSLNumericOrAggregateOfNumericType(clang::QualType type); bool IsHLSLCopyableAnnotatableRecord(clang::QualType QT); bool IsHLSLBuiltinRayAttributeStruct(clang::QualType QT); bool IsHLSLAggregateType(clang::QualType type); -hlsl::DXIL::ResourceClass GetHLSLResourceClass(clang::QualType type); clang::QualType GetHLSLResourceResultType(clang::QualType type); clang::QualType GetHLSLNodeIOResultType(clang::ASTContext &astContext, clang::QualType type); diff --git a/tools/clang/lib/AST/HlslTypes.cpp b/tools/clang/lib/AST/HlslTypes.cpp index 9748e13069..00c18a81a9 100644 --- a/tools/clang/lib/AST/HlslTypes.cpp +++ b/tools/clang/lib/AST/HlslTypes.cpp @@ -524,14 +524,6 @@ bool IsHLSLResourceType(clang::QualType type) { return false; } -hlsl::DXIL::ResourceClass GetHLSLResourceClass(clang::QualType type) { - - if (HLSLResourceAttr *attr = getAttr(type)) - return attr->getResClass(); - - return hlsl::DXIL::ResourceClass::Invalid; -} - bool IsHLSLHitObjectType(QualType type) { return nullptr != getAttr(type); } diff --git a/tools/clang/test/HLSL/rewriter/correct_rewrites/consistent_bindings_gold.hlsl b/tools/clang/test/HLSL/rewriter/correct_rewrites/consistent_bindings_gold.hlsl deleted file mode 100644 index 4c2810e9f4..0000000000 --- a/tools/clang/test/HLSL/rewriter/correct_rewrites/consistent_bindings_gold.hlsl +++ /dev/null @@ -1,49 +0,0 @@ -RWByteAddressBuffer output1 : register(u2); -RWByteAddressBuffer output2 : register(u15); -RWByteAddressBuffer output3 : register(u0); -RWByteAddressBuffer output4 : register(u0, space1); -RWByteAddressBuffer output5 : SEMA : register(u16); -RWByteAddressBuffer output6 : register(u17); -RWByteAddressBuffer output7 : register(u1); -RWByteAddressBuffer output8[12] : register(u3); -RWByteAddressBuffer output9[12] : register(u18); -RWByteAddressBuffer output10[33] : register(u1, space1); -RWByteAddressBuffer output11[33] : register(u33, space2); -RWByteAddressBuffer output12[33] : register(u0, space2); -StructuredBuffer test : register(t1); -ByteAddressBuffer input13 : SEMA : register(t2); -ByteAddressBuffer input14 : register(t15); -ByteAddressBuffer input15 : register(t0); -ByteAddressBuffer input16[12] : register(t3); -ByteAddressBuffer input17[2] : register(t13, space1); -ByteAddressBuffer input18[12] : register(t1, space1); -ByteAddressBuffer input19[3] : register(t15, space1); -ByteAddressBuffer input20 : register(t0, space1); -SamplerState sampler0 : register(s1); -SamplerState sampler1 : register(s2); -SamplerState sampler2 : register(s0); -SamplerState sampler3 : register(s1, space1); -SamplerState sampler4 : register(s0, space1); -cbuffer test : register(b0) { - const float a; -} -; -cbuffer test2 : register(b1) { - const float b; -} -; -cbuffer test3 : register(b0, space1) { - const float c; -} -; -cbuffer test4 : register(b1, space1) { - const float d; -} -; -const float e; -[numthreads(16, 16, 1)] -void main(uint id : SV_DispatchThreadID) { - output2.Store(id * 4, 1); -} - - diff --git a/tools/clang/tools/libclang/dxcrewriteunused.cpp b/tools/clang/tools/libclang/dxcrewriteunused.cpp index 23496242a4..6bc0135057 100644 --- a/tools/clang/tools/libclang/dxcrewriteunused.cpp +++ b/tools/clang/tools/libclang/dxcrewriteunused.cpp @@ -11,7 +11,6 @@ #include "clang/AST/ASTConsumer.h" #include "clang/AST/ASTContext.h" -#include "clang/AST/HlslTypes.h" #include "clang/AST/RecursiveASTVisitor.h" #include "clang/Basic/Diagnostic.h" #include "clang/Basic/FileManager.h" @@ -1085,6 +1084,7 @@ static HRESULT DoSimpleReWrite(DxcLangExtensionsHelper *pHelper, if (FAILED(hr)) return hr; + } else { o << "// Rewrite unchanged result:\n"; } diff --git a/tools/clang/unittests/HLSL/RewriterTest.cpp b/tools/clang/unittests/HLSL/RewriterTest.cpp index 1130772ff0..613c8561a3 100644 --- a/tools/clang/unittests/HLSL/RewriterTest.cpp +++ b/tools/clang/unittests/HLSL/RewriterTest.cpp @@ -126,19 +126,16 @@ class RewriterTest : public ::testing::Test { ppBlob)); } - VerifyResult CheckVerifies(LPCWSTR path, LPCWSTR goldPath, - const llvm::SmallVector &args = { - L"-HV", L"2016"}) { + VerifyResult CheckVerifies(LPCWSTR path, LPCWSTR goldPath) { CComPtr pRewriter; VERIFY_SUCCEEDED(CreateRewriter(&pRewriter)); - return CheckVerifies(pRewriter, path, goldPath, args); + return CheckVerifies(pRewriter, path, goldPath); } - VerifyResult - CheckVerifies(IDxcRewriter *pRewriter, LPCWSTR path, LPCWSTR goldPath, - const llvm::SmallVector &args = {L"-HV", L"2016"}) { + VerifyResult CheckVerifies(IDxcRewriter *pRewriter, LPCWSTR path, + LPCWSTR goldPath) { CComPtr pRewriteResult; - RewriteCompareGold(path, goldPath, &pRewriteResult, pRewriter, args); + RewriteCompareGold(path, goldPath, &pRewriteResult, pRewriter); VerifyResult toReturn; @@ -168,11 +165,9 @@ class RewriterTest : public ::testing::Test { return S_OK; } - VerifyResult CheckVerifiesHLSL(LPCWSTR name, LPCWSTR goldName, - const llvm::SmallVector &args = { - L"-HV", L"2016"}) { + VerifyResult CheckVerifiesHLSL(LPCWSTR name, LPCWSTR goldName) { return CheckVerifies(GetPathToHlslDataFile(name).c_str(), - GetPathToHlslDataFile(goldName).c_str(), args); + GetPathToHlslDataFile(goldName).c_str()); } struct FileWithBlob { @@ -215,8 +210,7 @@ class RewriterTest : public ::testing::Test { void RewriteCompareGold(LPCWSTR path, LPCWSTR goldPath, IDxcOperationResult **ppResult, - IDxcRewriter *rewriter, - const llvm::SmallVector &args = {}) { + IDxcRewriter *rewriter) { // Get the source text from a file FileWithBlob source(m_dllSupport, path); @@ -224,12 +218,14 @@ class RewriterTest : public ::testing::Test { DxcDefine myDefines[myDefinesCount] = { {L"myDefine", L"2"}, {L"myDefine3", L"1994"}, {L"myDefine4", nullptr}}; + LPCWSTR args[] = {L"-HV", L"2016"}; + CComPtr rewriter2; VERIFY_SUCCEEDED(rewriter->QueryInterface(&rewriter2)); // Run rewrite unchanged on the source code VERIFY_SUCCEEDED(rewriter2->RewriteWithOptions( - source.BlobEncoding, path, (LPCWSTR *)args.data(), - (uint32_t)args.size(), myDefines, myDefinesCount, nullptr, ppResult)); + source.BlobEncoding, path, args, _countof(args), myDefines, + myDefinesCount, nullptr, ppResult)); // check for compilation errors HRESULT hrStatus; From 8d7dc86e4e49dd1107f37fe456432ebfd647968f Mon Sep 17 00:00:00 2001 From: Nielsbishere Date: Tue, 2 Sep 2025 01:01:39 +0200 Subject: [PATCH 08/10] Cleanup for PR --- include/dxc/Support/HLSLOptions.h | 20 +++++++++---------- lib/HLSL/HLModule.cpp | 4 ++-- .../clang/tools/libclang/dxcrewriteunused.cpp | 1 - 3 files changed, 12 insertions(+), 13 deletions(-) diff --git a/include/dxc/Support/HLSLOptions.h b/include/dxc/Support/HLSLOptions.h index 10a41489c7..0c1bd8c846 100644 --- a/include/dxc/Support/HLSLOptions.h +++ b/include/dxc/Support/HLSLOptions.h @@ -102,16 +102,16 @@ class DxcDefines { }; struct RewriterOpts { - bool Unchanged = false; // OPT_rw_unchanged - bool SkipFunctionBody = false; // OPT_rw_skip_function_body - bool SkipStatic = false; // OPT_rw_skip_static - bool GlobalExternByDefault = false; // OPT_rw_global_extern_by_default - bool KeepUserMacro = false; // OPT_rw_keep_user_macro - bool ExtractEntryUniforms = false; // OPT_rw_extract_entry_uniforms - bool RemoveUnusedGlobals = false; // OPT_rw_remove_unused_globals - bool RemoveUnusedFunctions = false; // OPT_rw_remove_unused_functions - bool WithLineDirective = false; // OPT_rw_line_directive - bool DeclGlobalCB = false; // OPT_rw_decl_global_cb + bool Unchanged = false; // OPT_rw_unchanged + bool SkipFunctionBody = false; // OPT_rw_skip_function_body + bool SkipStatic = false; // OPT_rw_skip_static + bool GlobalExternByDefault = false; // OPT_rw_global_extern_by_default + bool KeepUserMacro = false; // OPT_rw_keep_user_macro + bool ExtractEntryUniforms = false; // OPT_rw_extract_entry_uniforms + bool RemoveUnusedGlobals = false; // OPT_rw_remove_unused_globals + bool RemoveUnusedFunctions = false; // OPT_rw_remove_unused_functions + bool WithLineDirective = false; // OPT_rw_line_directive + bool DeclGlobalCB = false; // OPT_rw_decl_global_cb }; /// Use this class to capture all options. diff --git a/lib/HLSL/HLModule.cpp b/lib/HLSL/HLModule.cpp index ce6241a3b6..f8cc85b82f 100644 --- a/lib/HLSL/HLModule.cpp +++ b/lib/HLSL/HLModule.cpp @@ -231,12 +231,12 @@ void HLModule::RemoveFunction(llvm::Function *F) { namespace { template bool RemoveResource(std::vector> &vec, - GlobalVariable *pVariable, bool keepAllocated, bool dontUpdateIds) { + GlobalVariable *pVariable, bool keepAllocated, bool consistentBindings) { for (auto p = vec.begin(), e = vec.end(); p != e; ++p) { if ((*p)->GetGlobalSymbol() != pVariable) continue; - if ((keepAllocated && (*p)->IsAllocated()) || dontUpdateIds) { + if ((keepAllocated && (*p)->IsAllocated()) || consistentBindings) { // Keep the resource, but it has no more symbol. (*p)->SetGlobalSymbol(UndefValue::get(pVariable->getType())); } else { diff --git a/tools/clang/tools/libclang/dxcrewriteunused.cpp b/tools/clang/tools/libclang/dxcrewriteunused.cpp index 6bc0135057..c29854077b 100644 --- a/tools/clang/tools/libclang/dxcrewriteunused.cpp +++ b/tools/clang/tools/libclang/dxcrewriteunused.cpp @@ -1083,7 +1083,6 @@ static HRESULT DoSimpleReWrite(DxcLangExtensionsHelper *pHelper, opts.RWOpt.RemoveUnusedFunctions, w); if (FAILED(hr)) return hr; - } else { o << "// Rewrite unchanged result:\n"; } From 298ff34d7c65df1386b52fafeee61c2a2f426f07 Mon Sep 17 00:00:00 2001 From: Nielsbishere Date: Tue, 2 Sep 2025 01:03:31 +0200 Subject: [PATCH 09/10] clang format --- lib/HLSL/HLModule.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/HLSL/HLModule.cpp b/lib/HLSL/HLModule.cpp index f8cc85b82f..8133317012 100644 --- a/lib/HLSL/HLModule.cpp +++ b/lib/HLSL/HLModule.cpp @@ -231,7 +231,8 @@ void HLModule::RemoveFunction(llvm::Function *F) { namespace { template bool RemoveResource(std::vector> &vec, - GlobalVariable *pVariable, bool keepAllocated, bool consistentBindings) { + GlobalVariable *pVariable, bool keepAllocated, + bool consistentBindings) { for (auto p = vec.begin(), e = vec.end(); p != e; ++p) { if ((*p)->GetGlobalSymbol() != pVariable) continue; From 28b24f11aaf835f69a7da157f7c1727e01fe4414 Mon Sep 17 00:00:00 2001 From: Nielsbishere Date: Tue, 2 Sep 2025 01:19:43 +0200 Subject: [PATCH 10/10] Clang format again --- include/dxc/HLSL/HLModule.h | 3 +-- lib/HLSL/DxilCondenseResources.cpp | 2 +- lib/HLSL/HLModule.cpp | 3 ++- tools/clang/lib/CodeGen/CGHLSLMS.cpp | 3 +-- tools/clang/tools/dxcompiler/dxcompilerobj.cpp | 3 +-- 5 files changed, 6 insertions(+), 8 deletions(-) diff --git a/include/dxc/HLSL/HLModule.h b/include/dxc/HLSL/HLModule.h index 90d0c218cd..351366db6f 100644 --- a/include/dxc/HLSL/HLModule.h +++ b/include/dxc/HLSL/HLModule.h @@ -54,8 +54,7 @@ struct HLOptions { bDisableOptimizations(false), PackingStrategy(0), bUseMinPrecision(false), bDX9CompatMode(false), bFXCCompatMode(false), bLegacyResourceReservation(false), bForceZeroStoreLifetimes(false), - bConsistentBindings(false), - unused(0) {} + bConsistentBindings(false), unused(0) {} uint32_t GetHLOptionsRaw() const; void SetHLOptionsRaw(uint32_t data); unsigned bDefaultRowMajor : 1; diff --git a/lib/HLSL/DxilCondenseResources.cpp b/lib/HLSL/DxilCondenseResources.cpp index 18dd741cc8..e6f77913af 100644 --- a/lib/HLSL/DxilCondenseResources.cpp +++ b/lib/HLSL/DxilCondenseResources.cpp @@ -551,7 +551,7 @@ class DxilLowerCreateHandleForLib : public ModulePass { // Remove unused resources. if (!DM.GetConsistentBindings()) - DM.RemoveResourcesWithUnusedSymbols(); + DM.RemoveResourcesWithUnusedSymbols(); unsigned newResources = DM.GetCBuffers().size() + DM.GetUAVs().size() + DM.GetSRVs().size() + DM.GetSamplers().size(); diff --git a/lib/HLSL/HLModule.cpp b/lib/HLSL/HLModule.cpp index 8133317012..41cafb971d 100644 --- a/lib/HLSL/HLModule.cpp +++ b/lib/HLSL/HLModule.cpp @@ -266,7 +266,8 @@ void HLModule::RemoveGlobal(llvm::GlobalVariable *GV) { // Consistent bindings are different than -flegacy-resource-reservation; // We need the IDs to stay the same, but it's fine to remove unused registers. - // It's actually wanted, because that allows us to know what registers are optimized out. + // It's actually wanted, because that allows us to know what registers are + // optimized out. bool consistentBindings = GetHLOptions().bConsistentBindings; // This could be considerably faster - check variable type to see which diff --git a/tools/clang/lib/CodeGen/CGHLSLMS.cpp b/tools/clang/lib/CodeGen/CGHLSLMS.cpp index 92964ba233..10824ff96a 100644 --- a/tools/clang/lib/CodeGen/CGHLSLMS.cpp +++ b/tools/clang/lib/CodeGen/CGHLSLMS.cpp @@ -400,8 +400,7 @@ CGMSHLSLRuntime::CGMSHLSLRuntime(CodeGenModule &CGM) opts.PackingStrategy = CGM.getCodeGenOpts().HLSLSignaturePackingStrategy; opts.bLegacyResourceReservation = CGM.getCodeGenOpts().HLSLLegacyResourceReservation; - opts.bConsistentBindings = - CGM.getCodeGenOpts().HLSLConsistentBindings; + opts.bConsistentBindings = CGM.getCodeGenOpts().HLSLConsistentBindings; opts.bForceZeroStoreLifetimes = CGM.getCodeGenOpts().HLSLForceZeroStoreLifetimes; diff --git a/tools/clang/tools/dxcompiler/dxcompilerobj.cpp b/tools/clang/tools/dxcompiler/dxcompilerobj.cpp index 9989aa4150..fee91145a5 100644 --- a/tools/clang/tools/dxcompiler/dxcompilerobj.cpp +++ b/tools/clang/tools/dxcompiler/dxcompilerobj.cpp @@ -1602,8 +1602,7 @@ class DxcCompiler : public IDxcCompiler3, compiler.getCodeGenOpts().HLSLAvoidControlFlow = Opts.AvoidFlowControl; compiler.getCodeGenOpts().HLSLLegacyResourceReservation = Opts.LegacyResourceReservation; - compiler.getCodeGenOpts().HLSLConsistentBindings = - Opts.ConsistentBindings; + compiler.getCodeGenOpts().HLSLConsistentBindings = Opts.ConsistentBindings; compiler.getCodeGenOpts().HLSLDefines = defines; compiler.getCodeGenOpts().HLSLPreciseOutputs = Opts.PreciseOutputs; compiler.getCodeGenOpts().MainFileName = pMainFile;