From 9887b09658c210d12efe0984d77adcee9ea99b3f Mon Sep 17 00:00:00 2001 From: Nielsbishere Date: Mon, 14 Jul 2025 22:36:30 +0200 Subject: [PATCH 01/29] 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/29] 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/29] 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/29] 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/29] 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/29] 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/29] 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/29] 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/29] 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/29] 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; From cb9f34586980a796b85dca04b8914fd515bdc096 Mon Sep 17 00:00:00 2001 From: Nielsbishere Date: Wed, 5 Nov 2025 23:39:47 +0100 Subject: [PATCH 11/29] Added two unit tests; one for more complex scenarios (e.g. overlapping registers, $Globals, samplers, readonly, readwrite, etc.) --- .../consistent_bindings_complex.hlsl | 190 ++++++++++++++++++ .../consistent_bindings_simple.hlsl | 58 ++++++ 2 files changed, 248 insertions(+) create mode 100644 tools/clang/test/HLSLFileCheck/d3dreflect/consistent_bindings_complex.hlsl create mode 100644 tools/clang/test/HLSLFileCheck/d3dreflect/consistent_bindings_simple.hlsl diff --git a/tools/clang/test/HLSLFileCheck/d3dreflect/consistent_bindings_complex.hlsl b/tools/clang/test/HLSLFileCheck/d3dreflect/consistent_bindings_complex.hlsl new file mode 100644 index 0000000000..e90274d5fa --- /dev/null +++ b/tools/clang/test/HLSLFileCheck/d3dreflect/consistent_bindings_complex.hlsl @@ -0,0 +1,190 @@ +// RUN: %dxc -T lib_6_3 -auto-binding-space 0 -consistent-bindings %s | %D3DReflect %s | FileCheck %s + +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); + +StructuredBuffer test5; +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); +Texture2D tex; + +SamplerState sampler0; +SamplerState sampler1; +SamplerState sampler2 : register(s0); +SamplerState sampler3 : register(space1); +SamplerState sampler4 : register(s0, space1); + +cbuffer test : register(b0) { float a; }; +cbuffer test2 { float b; }; +cbuffer test3 : register(space1) { float c; }; +cbuffer test4 : register(space1) { float d; }; + +float e; //$Global is always the first to receive bindings before cbuffers without register annotation + +[shader("compute")] +[numthreads(16, 16, 1)] +void main(uint id : SV_DispatchThreadID) { + output1.Store(0, test5[0]); + output2.Store(id * 4, a * b * c * d * e); //Only use 1 output, but this won't result into output2 receiving wrong bindings + output3.Store(0, input13.Load(0)); + output4.Store(0, input14.Load(0)); + output5.Store(0, input15.Load(0)); + output6.Store(0, input16[0].Load(0)); + output7.Store(0, input17[0].Load(0)); + output8[0].Store(0, input18[0].Load(0)); + output9[0].Store(0, input19[0].Load(0)); + output10[0].Store(0, input20.Load(0)); + output11[0].Store(0, tex.SampleLevel(sampler0, 0.xx, 0)); + output12[0].Store(0, tex.SampleLevel(sampler1, 0.xx, 0) * tex.SampleLevel(sampler2, 0.xx, 0) * tex.SampleLevel(sampler3, 0.xx, 0) * tex.SampleLevel(sampler4, 0.xx, 0)); +} + +// CHECK: ID3D12LibraryReflection: +// CHECK: D3D12_LIBRARY_DESC: +// CHECK: FunctionCount: 1 +// CHECK: ID3D12FunctionReflection: +// CHECK: D3D12_FUNCTION_DESC: Name: main +// CHECK: Shader Version: Compute 6.3 +// CHECK: BoundResources: 32 +// CHECK: Bound Resources: +// CHECK: D3D12_SHADER_INPUT_BIND_DESC: Name: $Globals +// CHECK: Type: D3D_SIT_CBUFFER +// CHECK: BindPoint: 1 +// CHECK: Space: 0 +// CHECK: D3D12_SHADER_INPUT_BIND_DESC: Name: test +// CHECK: Type: D3D_SIT_CBUFFER +// CHECK: BindPoint: 0 +// CHECK: Space: 0 +// CHECK: D3D12_SHADER_INPUT_BIND_DESC: Name: test2 +// CHECK: Type: D3D_SIT_CBUFFER +// CHECK: BindPoint: 2 +// CHECK: Space: 0 +// CHECK: D3D12_SHADER_INPUT_BIND_DESC: Name: test3 +// CHECK: Type: D3D_SIT_CBUFFER +// CHECK: BindPoint: 0 +// CHECK: Space: 1 +// CHECK: D3D12_SHADER_INPUT_BIND_DESC: Name: test4 +// CHECK: Type: D3D_SIT_CBUFFER +// CHECK: BindPoint: 1 +// CHECK: Space: 1 +// CHECK: D3D12_SHADER_INPUT_BIND_DESC: Name: sampler0 +// CHECK: Type: D3D_SIT_SAMPLER +// CHECK: BindPoint: 1 +// CHECK: Space: 0 +// CHECK: D3D12_SHADER_INPUT_BIND_DESC: Name: sampler1 +// CHECK: Type: D3D_SIT_SAMPLER +// CHECK: BindPoint: 2 +// CHECK: Space: 0 +// CHECK: D3D12_SHADER_INPUT_BIND_DESC: Name: sampler2 +// CHECK: Type: D3D_SIT_SAMPLER +// CHECK: BindPoint: 0 +// CHECK: Space: 0 +// CHECK: D3D12_SHADER_INPUT_BIND_DESC: Name: sampler3 +// CHECK: Type: D3D_SIT_SAMPLER +// CHECK: BindPoint: 1 +// CHECK: Space: 1 +// CHECK: D3D12_SHADER_INPUT_BIND_DESC: Name: sampler4 +// CHECK: Type: D3D_SIT_SAMPLER +// CHECK: BindPoint: 0 +// CHECK: Space: 1 +// CHECK: D3D12_SHADER_INPUT_BIND_DESC: Name: test5 +// CHECK: Type: D3D_SIT_STRUCTURED +// CHECK: BindPoint: 1 +// CHECK: Space: 0 +// CHECK: D3D12_SHADER_INPUT_BIND_DESC: Name: input13 +// CHECK: Type: D3D_SIT_BYTEADDRESS +// CHECK: BindPoint: 2 +// CHECK: Space: 0 +// CHECK: D3D12_SHADER_INPUT_BIND_DESC: Name: input14 +// CHECK: Type: D3D_SIT_BYTEADDRESS +// CHECK: BindPoint: 15 +// CHECK: Space: 0 +// CHECK: D3D12_SHADER_INPUT_BIND_DESC: Name: input15 +// CHECK: Type: D3D_SIT_BYTEADDRESS +// CHECK: BindPoint: 0 +// CHECK: Space: 0 +// CHECK: D3D12_SHADER_INPUT_BIND_DESC: Name: input16 +// CHECK: Type: D3D_SIT_BYTEADDRESS +// CHECK: BindPoint: 3 +// CHECK: Space: 0 +// CHECK: D3D12_SHADER_INPUT_BIND_DESC: Name: input17 +// CHECK: Type: D3D_SIT_BYTEADDRESS +// CHECK: BindPoint: 13 +// CHECK: Space: 1 +// CHECK: D3D12_SHADER_INPUT_BIND_DESC: Name: input18 +// CHECK: Type: D3D_SIT_BYTEADDRESS +// CHECK: BindPoint: 1 +// CHECK: Space: 1 +// CHECK: D3D12_SHADER_INPUT_BIND_DESC: Name: input19 +// CHECK: Type: D3D_SIT_BYTEADDRESS +// CHECK: BindPoint: 15 +// CHECK: Space: 1 +// CHECK: D3D12_SHADER_INPUT_BIND_DESC: Name: input20 +// CHECK: Type: D3D_SIT_BYTEADDRESS +// CHECK: BindPoint: 0 +// CHECK: Space: 1 +// CHECK: D3D12_SHADER_INPUT_BIND_DESC: Name: tex +// CHECK: BindPoint: 16 +// CHECK: Space: 0 +// CHECK: D3D12_SHADER_INPUT_BIND_DESC: Name: output1 +// CHECK: Type: D3D_SIT_UAV_RWBYTEADDRESS +// CHECK: BindPoint: 2 +// CHECK: Space: 0 +// CHECK: D3D12_SHADER_INPUT_BIND_DESC: Name: output2 +// CHECK: Type: D3D_SIT_UAV_RWBYTEADDRESS +// CHECK: BindPoint: 15 +// CHECK: Space: 0 +// CHECK: D3D12_SHADER_INPUT_BIND_DESC: Name: output3 +// CHECK: Type: D3D_SIT_UAV_RWBYTEADDRESS +// CHECK: BindPoint: 0 +// CHECK: Space: 0 +// CHECK: D3D12_SHADER_INPUT_BIND_DESC: Name: output4 +// CHECK: BindPoint: 0 +// CHECK: Space: 1 +// CHECK: D3D12_SHADER_INPUT_BIND_DESC: Name: output5 +// CHECK: Type: D3D_SIT_UAV_RWBYTEADDRESS +// CHECK: BindPoint: 16 +// CHECK: Space: 0 +// CHECK: D3D12_SHADER_INPUT_BIND_DESC: Name: output6 +// CHECK: Type: D3D_SIT_UAV_RWBYTEADDRESS +// CHECK: BindPoint: 17 +// CHECK: Space: 0 +// CHECK: D3D12_SHADER_INPUT_BIND_DESC: Name: output7 +// CHECK: Type: D3D_SIT_UAV_RWBYTEADDRESS +// CHECK: BindPoint: 1 +// CHECK: Space: 0 +// CHECK: D3D12_SHADER_INPUT_BIND_DESC: Name: output8 +// CHECK: Type: D3D_SIT_UAV_RWBYTEADDRESS +// CHECK: BindPoint: 3 +// CHECK: Space: 0 +// CHECK: D3D12_SHADER_INPUT_BIND_DESC: Name: output9 +// CHECK: Type: D3D_SIT_UAV_RWBYTEADDRESS +// CHECK: BindPoint: 18 +// CHECK: Space: 0 +// CHECK: D3D12_SHADER_INPUT_BIND_DESC: Name: output10 +// CHECK: Type: D3D_SIT_UAV_RWBYTEADDRESS +// CHECK: BindPoint: 1 +// CHECK: Space: 1 +// CHECK: D3D12_SHADER_INPUT_BIND_DESC: Name: output11 +// CHECK: Type: D3D_SIT_UAV_RWBYTEADDRESS +// CHECK: BindPoint: 33 +// CHECK: Space: 2 +// CHECK: D3D12_SHADER_INPUT_BIND_DESC: Name: output12 +// CHECK: Type: D3D_SIT_UAV_RWBYTEADDRESS +// CHECK: BindPoint: 0 +// CHECK: Space: 2 \ No newline at end of file diff --git a/tools/clang/test/HLSLFileCheck/d3dreflect/consistent_bindings_simple.hlsl b/tools/clang/test/HLSLFileCheck/d3dreflect/consistent_bindings_simple.hlsl new file mode 100644 index 0000000000..1d7ffbb6d8 --- /dev/null +++ b/tools/clang/test/HLSLFileCheck/d3dreflect/consistent_bindings_simple.hlsl @@ -0,0 +1,58 @@ +// RUN: %dxc -T lib_6_3 -auto-binding-space 0 -consistent-bindings %s | %D3DReflect %s | FileCheck %s -check-prefix=CHECK +// RUN: %dxc -T cs_6_3 -E mainA -auto-binding-space 0 -consistent-bindings %s | %D3DReflect %s | FileCheck %s -check-prefix=CHECK2 +// RUN: %dxc -T cs_6_3 -E mainB -auto-binding-space 0 -consistent-bindings %s | %D3DReflect %s | FileCheck %s -check-prefix=CHECK3 + +RWByteAddressBuffer a; +RWByteAddressBuffer b; + +[shader("compute")] +[numthreads(16, 16, 1)] +void mainA(uint id : SV_DispatchThreadID) { + a.Store(0, float4(id, 0.xxx)); +} + +[shader("compute")] +[numthreads(16, 16, 1)] +void mainB(uint id : SV_DispatchThreadID) { + b.Store(0, float4(id, 0.xxx)); +} + +// CHECK: ID3D12LibraryReflection: +// CHECK: D3D12_LIBRARY_DESC: +// CHECK: FunctionCount: 2 +// CHECK: ID3D12FunctionReflection: +// CHECK: D3D12_FUNCTION_DESC: Name: mainA +// CHECK: Shader Version: Compute 6.3 +// CHECK: BoundResources: 1 +// CHECK: Bound Resources: +// CHECK: D3D12_SHADER_INPUT_BIND_DESC: Name: a +// CHECK: Type: D3D_SIT_UAV_RWBYTEADDRESS +// CHECK: BindPoint: 0 +// CHECK: Space: 0 +// CHECK: ID3D12FunctionReflection: +// CHECK: D3D12_FUNCTION_DESC: Name: mainB +// CHECK: Shader Version: Compute 6.3 +// CHECK: BoundResources: 1 +// CHECK: Bound Resources: +// CHECK: D3D12_SHADER_INPUT_BIND_DESC: Name: b +// CHECK: Type: D3D_SIT_UAV_RWBYTEADDRESS +// CHECK: BindPoint: 1 +// CHECK: Space: 0 + +// CHECK2: ID3D12ShaderReflection: +// CHECK2: D3D12_SHADER_DESC: +// CHECK2: Shader Version: Compute +// CHECK2: Bound Resources: +// CHECK2: D3D12_SHADER_INPUT_BIND_DESC: Name: a +// CHECK2: Type: D3D_SIT_UAV_RWBYTEADDRESS +// CHECK2: BindPoint: 0 +// CHECK2: Space: 0 + +// CHECK3: ID3D12ShaderReflection: +// CHECK3: D3D12_SHADER_DESC: +// CHECK3: Shader Version: Compute +// CHECK3: Bound Resources: +// CHECK3: D3D12_SHADER_INPUT_BIND_DESC: Name: b +// CHECK3: Type: D3D_SIT_UAV_RWBYTEADDRESS +// CHECK3: BindPoint: 1 +// CHECK3: Space: 0 \ No newline at end of file From 777a50e6d5405c2ce4ca3d9b007448ed7824e134 Mon Sep 17 00:00:00 2001 From: Nielsbishere Date: Thu, 6 Nov 2025 13:29:43 +0100 Subject: [PATCH 12/29] RemoveResourcesWithUnusedSymbolsHelper and RemoveResourcesWithUnusedSymbols now return bool isModified. Passing isModified along from RemoveResourcesWithUnusedSymbols to bChanged and fixed the LegalizeResources + -consistent-bindings oversight. --- include/dxc/DXIL/DxilModule.h | 2 +- lib/DXIL/DxilModule.cpp | 15 +++++++++------ lib/HLSL/DxilCondenseResources.cpp | 12 ++++++------ 3 files changed, 16 insertions(+), 13 deletions(-) diff --git a/include/dxc/DXIL/DxilModule.h b/include/dxc/DXIL/DxilModule.h index 5a91f0b204..ee5bd28f3a 100644 --- a/include/dxc/DXIL/DxilModule.h +++ b/include/dxc/DXIL/DxilModule.h @@ -112,7 +112,7 @@ class DxilModule { const std::vector> &GetUAVs() const; void RemoveUnusedResources(); - void RemoveResourcesWithUnusedSymbols(); + bool RemoveResourcesWithUnusedSymbols(); void RemoveFunction(llvm::Function *F); bool RenameResourcesWithPrefix(const std::string &prefix); diff --git a/lib/DXIL/DxilModule.cpp b/lib/DXIL/DxilModule.cpp index d55923d964..e98b67f56e 100644 --- a/lib/DXIL/DxilModule.cpp +++ b/lib/DXIL/DxilModule.cpp @@ -1031,7 +1031,7 @@ void DxilModule::RemoveUnusedResources() { namespace { template -static void RemoveResourcesWithUnusedSymbolsHelper( +static bool RemoveResourcesWithUnusedSymbolsHelper( std::vector> &vec) { unsigned resID = 0; std::unordered_set @@ -1054,14 +1054,17 @@ static void RemoveResourcesWithUnusedSymbolsHelper( for (auto gv : eraseList) { gv->eraseFromParent(); } + return !eraseList.empty(); } } // namespace -void DxilModule::RemoveResourcesWithUnusedSymbols() { - RemoveResourcesWithUnusedSymbolsHelper(m_SRVs); - RemoveResourcesWithUnusedSymbolsHelper(m_UAVs); - RemoveResourcesWithUnusedSymbolsHelper(m_CBuffers); - RemoveResourcesWithUnusedSymbolsHelper(m_Samplers); +bool DxilModule::RemoveResourcesWithUnusedSymbols() { + bool modif = false; + modif |= RemoveResourcesWithUnusedSymbolsHelper(m_SRVs); + modif |= RemoveResourcesWithUnusedSymbolsHelper(m_UAVs); + modif |= RemoveResourcesWithUnusedSymbolsHelper(m_CBuffers); + modif |= RemoveResourcesWithUnusedSymbolsHelper(m_Samplers); + return modif; } namespace { diff --git a/lib/HLSL/DxilCondenseResources.cpp b/lib/HLSL/DxilCondenseResources.cpp index e6f77913af..1376db491f 100644 --- a/lib/HLSL/DxilCondenseResources.cpp +++ b/lib/HLSL/DxilCondenseResources.cpp @@ -551,11 +551,11 @@ class DxilLowerCreateHandleForLib : public ModulePass { // Remove unused resources. if (!DM.GetConsistentBindings()) - DM.RemoveResourcesWithUnusedSymbols(); + bChanged |= DM.RemoveResourcesWithUnusedSymbols(); unsigned newResources = DM.GetCBuffers().size() + DM.GetUAVs().size() + DM.GetSRVs().size() + DM.GetSamplers().size(); - bChanged = bChanged || (numResources != newResources); + bChanged |= numResources != newResources; if (0 == newResources) return bChanged; @@ -563,9 +563,9 @@ class DxilLowerCreateHandleForLib : public ModulePass { { DxilValueCache *DVC = &getAnalysis(); bool bLocalChanged = LegalizeResources(M, DVC); - if (bLocalChanged) { + if (bLocalChanged && !DM.GetConsistentBindings()) { // Remove unused resources. - DM.RemoveResourcesWithUnusedSymbols(); + bChanged |= DM.RemoveResourcesWithUnusedSymbols(); } bChanged |= bLocalChanged; } @@ -573,8 +573,8 @@ class DxilLowerCreateHandleForLib : public ModulePass { bChanged |= ResourceRegisterAllocator.AllocateRegisters(DM); if (DM.GetConsistentBindings()) - DM.RemoveResourcesWithUnusedSymbols(); - + bChanged |= DM.RemoveResourcesWithUnusedSymbols(); + // Fill in top-level CBuffer variable usage bit UpdateCBufferUsage(); From 3fc2f2e1311c2e4ea7b4a392ae6bd85d03baf2c0 Mon Sep 17 00:00:00 2001 From: Nielsbishere Date: Thu, 6 Nov 2025 13:38:17 +0100 Subject: [PATCH 13/29] Applied clang format --- lib/HLSL/DxilCondenseResources.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/HLSL/DxilCondenseResources.cpp b/lib/HLSL/DxilCondenseResources.cpp index 1376db491f..c39dd7ef46 100644 --- a/lib/HLSL/DxilCondenseResources.cpp +++ b/lib/HLSL/DxilCondenseResources.cpp @@ -574,7 +574,7 @@ class DxilLowerCreateHandleForLib : public ModulePass { if (DM.GetConsistentBindings()) bChanged |= DM.RemoveResourcesWithUnusedSymbols(); - + // Fill in top-level CBuffer variable usage bit UpdateCBufferUsage(); From ea760bf95227ffb40285953ebeb221e9b3c33bf4 Mon Sep 17 00:00:00 2001 From: Nielsbishere Date: Fri, 7 Nov 2025 21:54:47 +0100 Subject: [PATCH 14/29] Fixed an issue where resources without a global variable would not properly trigger a modification --- lib/DXIL/DxilModule.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/lib/DXIL/DxilModule.cpp b/lib/DXIL/DxilModule.cpp index e98b67f56e..df491f7d4d 100644 --- a/lib/DXIL/DxilModule.cpp +++ b/lib/DXIL/DxilModule.cpp @@ -1033,6 +1033,7 @@ namespace { template static bool RemoveResourcesWithUnusedSymbolsHelper( std::vector> &vec) { + bool modif = false; unsigned resID = 0; std::unordered_set eraseList; // Need in case of duplicate defs of lib resources @@ -1044,6 +1045,7 @@ static bool RemoveResourcesWithUnusedSymbolsHelper( p = vec.erase(c); if (GlobalVariable *GV = dyn_cast(symbol)) eraseList.insert(GV); + modif = true; continue; } if ((*c)->GetID() != resID) { @@ -1054,7 +1056,7 @@ static bool RemoveResourcesWithUnusedSymbolsHelper( for (auto gv : eraseList) { gv->eraseFromParent(); } - return !eraseList.empty(); + return modif; } } // namespace From 9169ce03946f0bd7b9f0c6f3f48bffefbdc911d2 Mon Sep 17 00:00:00 2001 From: Nielsbishere Date: Wed, 12 Nov 2025 12:15:35 +0100 Subject: [PATCH 15/29] Added -fhlsl-unused-resource-bindings with options strip or reserve in this branch. Added release note --- docs/ReleaseNotes.md | 9 +++++---- include/dxc/Support/HLSLOptions.h | 7 +++++-- include/dxc/Support/HLSLOptions.td | 4 ++-- lib/DxcSupport/HLSLOptions.cpp | 17 +++++++++++++++-- tools/clang/tools/dxcompiler/dxcompilerobj.cpp | 5 +++-- 5 files changed, 30 insertions(+), 12 deletions(-) diff --git a/docs/ReleaseNotes.md b/docs/ReleaseNotes.md index 4fdb82f2ca..09495e0c46 100644 --- a/docs/ReleaseNotes.md +++ b/docs/ReleaseNotes.md @@ -23,6 +23,7 @@ The included licenses apply to the following files: - Fixed regression: [#7508](https://github.com/microsoft/DirectXShaderCompiler/issues/7508) crash when calling `Load` with `status`. - Header file `dxcpix.h` was added to the release package. - Moved Linear Algebra (Cooperative Vector) DXIL Opcodes to experimental Shader Model 6.10 +- Added `-fhlsl-unused-resource-bindings=` an option to allow deciding on how to treat unused resource bindings in DXIL; `strip` (default), `keep` or `reserve`. `strip` will strip unused resources before generating bindings for resources without a `: register`, `keep` keeps them around while marking them with `D3D_SIF_UNUSED` in reflection and `reserve` will keep them reserved for generated bindings while stripping them afterwards. See [explanation](https://github.com/microsoft/DirectXShaderCompiler/pull/7643#issuecomment-3496917202) for more details. ### Version 1.8.2505 @@ -182,11 +183,11 @@ DX Compiler release for December 2022. This includes the compiler executable, the dynamic library, and the dxil signing library. - New flags for inspecting compile times: - `-ftime-report` flag prints a high level summary of compile time broken down by major phase or pass in the compiler. The DXC -command line will print the output to stdout. + command line will print the output to stdout. - `-ftime-trace` flag prints a Chrome trace json file. The output can be routed to a specific file by providing a filename to -the argument using the format `-ftime-trace=`. Chrome trace files can be opened in Chrome by loading the built-in tracing tool -at chrome://tracing. The trace file captures hierarchial timing data with additional context enabling a much more in-depth profiling -experience. + the argument using the format `-ftime-trace=`. Chrome trace files can be opened in Chrome by loading the built-in tracing tool + at chrome://tracing. The trace file captures hierarchial timing data with additional context enabling a much more in-depth profiling + experience. - Both new options are supported via the DXC API using the `DXC_OUT_TIME_REPORT` and `DXC_OUT_TIME_TRACE` output kinds respectively. - IDxcPdbUtils2 enables reading new PDB container part - `-P` flag will now behave as it does with cl using the file specified by `-Fi` or a default diff --git a/include/dxc/Support/HLSLOptions.h b/include/dxc/Support/HLSLOptions.h index 0e778ae930..41a84b4b8a 100644 --- a/include/dxc/Support/HLSLOptions.h +++ b/include/dxc/Support/HLSLOptions.h @@ -54,6 +54,8 @@ enum HlslFlags { RewriteOption = (1 << 17), }; +enum class UnusedResourceBinding { Strip, Reserve, Keep }; + enum ID { OPT_INVALID = 0, // This is not an option ID. #define OPTION(PREFIX, NAME, ID, KIND, GROUP, ALIAS, ALIASARGS, FLAGS, PARAM, \ @@ -227,8 +229,9 @@ 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 - bool Verbose = false; // OPT_verbose + UnusedResourceBinding UnusedResources = + UnusedResourceBinding::Strip; // OPT_fhlsl_unused_resource_bindings_EQ + bool Verbose = false; // OPT_verbose // Optimization pass enables, disables and selects OptimizationToggles diff --git a/include/dxc/Support/HLSLOptions.td b/include/dxc/Support/HLSLOptions.td index d6edb6c03c..c38377f26d 100644 --- a/include/dxc/Support/HLSLOptions.td +++ b/include/dxc/Support/HLSLOptions.td @@ -576,8 +576,8 @@ 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).">; +def fhlsl_unused_resource_bindings_EQ : Joined<["-"], "fhlsl-unused-resource-bindings=">, Group, Flags<[CoreOption]>, + HelpText<"Control handling of unused resource bindings: 'strip' (default, generate bindings after optimization) or 'reserve'.">; ////////////////////////////////////////////////////////////////////////////// // Rewriter Options diff --git a/lib/DxcSupport/HLSLOptions.cpp b/lib/DxcSupport/HLSLOptions.cpp index fad3c81ffe..57a1d0c490 100644 --- a/lib/DxcSupport/HLSLOptions.cpp +++ b/lib/DxcSupport/HLSLOptions.cpp @@ -865,8 +865,21 @@ 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); + + std::string UnusedResources = + Args.getLastArgValue(OPT_fhlsl_unused_resource_bindings_EQ, "strip"); + + if (UnusedResources == "strip") + opts.UnusedResources = UnusedResourceBinding::Strip; + else if (UnusedResources == "reserve") + opts.UnusedResources = UnusedResourceBinding::Reserve; + else { + errors << "Error: Invalid value for -fhlsl-unused-resource-bindings option " + "specified (" + << UnusedResources << "). Must be one of: strip, reserve"; + return 1; + } + opts.Verbose = Args.hasFlag(OPT_verbose, OPT_INVALID, false); if (Args.hasArg(OPT_ftime_trace_EQ)) opts.TimeTrace = Args.getLastArgValue(OPT_ftime_trace_EQ); diff --git a/tools/clang/tools/dxcompiler/dxcompilerobj.cpp b/tools/clang/tools/dxcompiler/dxcompilerobj.cpp index 3a6af3ade1..6b956e0a04 100644 --- a/tools/clang/tools/dxcompiler/dxcompilerobj.cpp +++ b/tools/clang/tools/dxcompiler/dxcompilerobj.cpp @@ -1288,7 +1288,7 @@ class DxcCompiler : public IDxcCompiler3, IFT(pReserializeResult->GetResult(&pNewOutput)); pOutputBlob = pNewOutput; } // PDB in private - } // Write PDB + } // Write PDB IFT(primaryOutput.SetObject(pOutputBlob, opts.DefaultTextCodePage)); IFT(pResult->SetOutput(primaryOutput)); @@ -1595,7 +1595,8 @@ class DxcCompiler : public IDxcCompiler3, compiler.getCodeGenOpts().HLSLAvoidControlFlow = Opts.AvoidFlowControl; compiler.getCodeGenOpts().HLSLLegacyResourceReservation = Opts.LegacyResourceReservation; - compiler.getCodeGenOpts().HLSLConsistentBindings = Opts.ConsistentBindings; + compiler.getCodeGenOpts().HLSLConsistentBindings = + Opts.UnusedResources == hlsl::options::UnusedResourceBinding::Reserve; compiler.getCodeGenOpts().HLSLDefines = defines; compiler.getCodeGenOpts().HLSLPreciseOutputs = Opts.PreciseOutputs; compiler.getCodeGenOpts().MainFileName = pMainFile; From 6a15db9a721a3f42633ed97a2d378a4a8b3e0013 Mon Sep 17 00:00:00 2001 From: Nielsbishere Date: Wed, 12 Nov 2025 12:15:55 +0100 Subject: [PATCH 16/29] Updated unit tests --- .../d3dreflect/consistent_bindings_complex.hlsl | 2 +- .../d3dreflect/consistent_bindings_simple.hlsl | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/tools/clang/test/HLSLFileCheck/d3dreflect/consistent_bindings_complex.hlsl b/tools/clang/test/HLSLFileCheck/d3dreflect/consistent_bindings_complex.hlsl index e90274d5fa..4a936f445f 100644 --- a/tools/clang/test/HLSLFileCheck/d3dreflect/consistent_bindings_complex.hlsl +++ b/tools/clang/test/HLSLFileCheck/d3dreflect/consistent_bindings_complex.hlsl @@ -1,4 +1,4 @@ -// RUN: %dxc -T lib_6_3 -auto-binding-space 0 -consistent-bindings %s | %D3DReflect %s | FileCheck %s +// RUN: %dxc -T lib_6_3 -auto-binding-space 0 -fhlsl-unused-resource-bindings=reserve %s | %D3DReflect %s | FileCheck %s RWByteAddressBuffer output1; RWByteAddressBuffer output2; diff --git a/tools/clang/test/HLSLFileCheck/d3dreflect/consistent_bindings_simple.hlsl b/tools/clang/test/HLSLFileCheck/d3dreflect/consistent_bindings_simple.hlsl index 1d7ffbb6d8..c63c5fecd0 100644 --- a/tools/clang/test/HLSLFileCheck/d3dreflect/consistent_bindings_simple.hlsl +++ b/tools/clang/test/HLSLFileCheck/d3dreflect/consistent_bindings_simple.hlsl @@ -1,6 +1,6 @@ -// RUN: %dxc -T lib_6_3 -auto-binding-space 0 -consistent-bindings %s | %D3DReflect %s | FileCheck %s -check-prefix=CHECK -// RUN: %dxc -T cs_6_3 -E mainA -auto-binding-space 0 -consistent-bindings %s | %D3DReflect %s | FileCheck %s -check-prefix=CHECK2 -// RUN: %dxc -T cs_6_3 -E mainB -auto-binding-space 0 -consistent-bindings %s | %D3DReflect %s | FileCheck %s -check-prefix=CHECK3 +// RUN: %dxc -T lib_6_3 -auto-binding-space 0 -fhlsl-unused-resource-bindings=reserve %s | %D3DReflect %s | FileCheck %s -check-prefix=CHECK +// RUN: %dxc -T cs_6_3 -E mainA -auto-binding-space 0 -fhlsl-unused-resource-bindings=reserve %s | %D3DReflect %s | FileCheck %s -check-prefix=CHECK2 +// RUN: %dxc -T cs_6_3 -E mainB -auto-binding-space 0 -fhlsl-unused-resource-bindings=reserve %s | %D3DReflect %s | FileCheck %s -check-prefix=CHECK3 RWByteAddressBuffer a; RWByteAddressBuffer b; From a633dc65448828644d8a2fc79e7ebc47eaba778c Mon Sep 17 00:00:00 2001 From: Nielsbishere Date: Wed, 12 Nov 2025 12:41:04 +0100 Subject: [PATCH 17/29] Clang format that somehow doesn't get formatted correctly? --- tools/clang/tools/dxcompiler/dxcompilerobj.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/clang/tools/dxcompiler/dxcompilerobj.cpp b/tools/clang/tools/dxcompiler/dxcompilerobj.cpp index 6b956e0a04..438ed7bceb 100644 --- a/tools/clang/tools/dxcompiler/dxcompilerobj.cpp +++ b/tools/clang/tools/dxcompiler/dxcompilerobj.cpp @@ -1288,7 +1288,7 @@ class DxcCompiler : public IDxcCompiler3, IFT(pReserializeResult->GetResult(&pNewOutput)); pOutputBlob = pNewOutput; } // PDB in private - } // Write PDB + } // Write PDB IFT(primaryOutput.SetObject(pOutputBlob, opts.DefaultTextCodePage)); IFT(pResult->SetOutput(primaryOutput)); From 316e493628861585cca7bb79067a51b26899d1bc Mon Sep 17 00:00:00 2001 From: Nielsbishere Date: Thu, 13 Nov 2025 13:34:58 +0100 Subject: [PATCH 18/29] PR feedback, re. reserve-all, bChanged and revert formatting done to release notes done by my markdown editor. TODO: Use UnusedResourceBindings in HLModule, DxilModule and CodeGenOptions --- docs/ReleaseNotes.md | 10 +++++----- include/dxc/Support/HLSLOptions.h | 2 +- include/dxc/Support/HLSLOptions.td | 2 +- lib/DxcSupport/HLSLOptions.cpp | 6 +++--- lib/HLSL/DxilCondenseResources.cpp | 1 - .../d3dreflect/consistent_bindings_complex.hlsl | 2 +- .../d3dreflect/consistent_bindings_simple.hlsl | 6 +++--- tools/clang/tools/dxcompiler/dxcompilerobj.cpp | 2 +- 8 files changed, 15 insertions(+), 16 deletions(-) diff --git a/docs/ReleaseNotes.md b/docs/ReleaseNotes.md index 09495e0c46..8f7cb7685f 100644 --- a/docs/ReleaseNotes.md +++ b/docs/ReleaseNotes.md @@ -23,7 +23,7 @@ The included licenses apply to the following files: - Fixed regression: [#7508](https://github.com/microsoft/DirectXShaderCompiler/issues/7508) crash when calling `Load` with `status`. - Header file `dxcpix.h` was added to the release package. - Moved Linear Algebra (Cooperative Vector) DXIL Opcodes to experimental Shader Model 6.10 -- Added `-fhlsl-unused-resource-bindings=` an option to allow deciding on how to treat unused resource bindings in DXIL; `strip` (default), `keep` or `reserve`. `strip` will strip unused resources before generating bindings for resources without a `: register`, `keep` keeps them around while marking them with `D3D_SIF_UNUSED` in reflection and `reserve` will keep them reserved for generated bindings while stripping them afterwards. See [explanation](https://github.com/microsoft/DirectXShaderCompiler/pull/7643#issuecomment-3496917202) for more details. +- Added `-fhlsl-unused-resource-bindings=` an option to allow deciding on how to treat unused resource bindings in DXIL; `strip` (default) or `reserve-all`. `strip` will strip unused resources before generating bindings for resources without a `: register`, and `reserve-all` will keep them reserved for generated bindings while stripping them afterwards. See [explanation](https://github.com/microsoft/DirectXShaderCompiler/pull/7643#issuecomment-3496917202) for more details. ### Version 1.8.2505 @@ -183,11 +183,11 @@ DX Compiler release for December 2022. This includes the compiler executable, the dynamic library, and the dxil signing library. - New flags for inspecting compile times: - `-ftime-report` flag prints a high level summary of compile time broken down by major phase or pass in the compiler. The DXC - command line will print the output to stdout. +command line will print the output to stdout. - `-ftime-trace` flag prints a Chrome trace json file. The output can be routed to a specific file by providing a filename to - the argument using the format `-ftime-trace=`. Chrome trace files can be opened in Chrome by loading the built-in tracing tool - at chrome://tracing. The trace file captures hierarchial timing data with additional context enabling a much more in-depth profiling - experience. +the argument using the format `-ftime-trace=`. Chrome trace files can be opened in Chrome by loading the built-in tracing tool +at chrome://tracing. The trace file captures hierarchial timing data with additional context enabling a much more in-depth profiling +experience. - Both new options are supported via the DXC API using the `DXC_OUT_TIME_REPORT` and `DXC_OUT_TIME_TRACE` output kinds respectively. - IDxcPdbUtils2 enables reading new PDB container part - `-P` flag will now behave as it does with cl using the file specified by `-Fi` or a default diff --git a/include/dxc/Support/HLSLOptions.h b/include/dxc/Support/HLSLOptions.h index 41a84b4b8a..e42e2b81e8 100644 --- a/include/dxc/Support/HLSLOptions.h +++ b/include/dxc/Support/HLSLOptions.h @@ -54,7 +54,7 @@ enum HlslFlags { RewriteOption = (1 << 17), }; -enum class UnusedResourceBinding { Strip, Reserve, Keep }; +enum class UnusedResourceBinding { Strip, ReserveAll }; enum ID { OPT_INVALID = 0, // This is not an option ID. diff --git a/include/dxc/Support/HLSLOptions.td b/include/dxc/Support/HLSLOptions.td index c38377f26d..d8f8d9e49c 100644 --- a/include/dxc/Support/HLSLOptions.td +++ b/include/dxc/Support/HLSLOptions.td @@ -577,7 +577,7 @@ def nologo : Flag<["-", "/"], "nologo">, Group, Flags<[DriverOpt HelpText<"Suppress copyright message">; def fhlsl_unused_resource_bindings_EQ : Joined<["-"], "fhlsl-unused-resource-bindings=">, Group, Flags<[CoreOption]>, - HelpText<"Control handling of unused resource bindings: 'strip' (default, generate bindings after optimization) or 'reserve'.">; + HelpText<"Control handling of unused resource bindings:\n\t\t\t'strip' (default, unused resources are stripped and their binding slots are freed up).\n\t\t\t'reserve-all' (do not use binding slots of unused resources when assigning bindings).">; ////////////////////////////////////////////////////////////////////////////// // Rewriter Options diff --git a/lib/DxcSupport/HLSLOptions.cpp b/lib/DxcSupport/HLSLOptions.cpp index 57a1d0c490..1fbdca06d1 100644 --- a/lib/DxcSupport/HLSLOptions.cpp +++ b/lib/DxcSupport/HLSLOptions.cpp @@ -871,12 +871,12 @@ int ReadDxcOpts(const OptTable *optionTable, unsigned flagsToInclude, if (UnusedResources == "strip") opts.UnusedResources = UnusedResourceBinding::Strip; - else if (UnusedResources == "reserve") - opts.UnusedResources = UnusedResourceBinding::Reserve; + else if (UnusedResources == "reserve-all") + opts.UnusedResources = UnusedResourceBinding::ReserveAll; else { errors << "Error: Invalid value for -fhlsl-unused-resource-bindings option " "specified (" - << UnusedResources << "). Must be one of: strip, reserve"; + << UnusedResources << "). Must be one of: strip, reserve-all"; return 1; } diff --git a/lib/HLSL/DxilCondenseResources.cpp b/lib/HLSL/DxilCondenseResources.cpp index c39dd7ef46..824a9b4c4f 100644 --- a/lib/HLSL/DxilCondenseResources.cpp +++ b/lib/HLSL/DxilCondenseResources.cpp @@ -555,7 +555,6 @@ class DxilLowerCreateHandleForLib : public ModulePass { unsigned newResources = DM.GetCBuffers().size() + DM.GetUAVs().size() + DM.GetSRVs().size() + DM.GetSamplers().size(); - bChanged |= numResources != newResources; if (0 == newResources) return bChanged; diff --git a/tools/clang/test/HLSLFileCheck/d3dreflect/consistent_bindings_complex.hlsl b/tools/clang/test/HLSLFileCheck/d3dreflect/consistent_bindings_complex.hlsl index 4a936f445f..2aef348bd9 100644 --- a/tools/clang/test/HLSLFileCheck/d3dreflect/consistent_bindings_complex.hlsl +++ b/tools/clang/test/HLSLFileCheck/d3dreflect/consistent_bindings_complex.hlsl @@ -1,4 +1,4 @@ -// RUN: %dxc -T lib_6_3 -auto-binding-space 0 -fhlsl-unused-resource-bindings=reserve %s | %D3DReflect %s | FileCheck %s +// RUN: %dxc -T lib_6_3 -auto-binding-space 0 -fhlsl-unused-resource-bindings=reserve-all %s | %D3DReflect %s | FileCheck %s RWByteAddressBuffer output1; RWByteAddressBuffer output2; diff --git a/tools/clang/test/HLSLFileCheck/d3dreflect/consistent_bindings_simple.hlsl b/tools/clang/test/HLSLFileCheck/d3dreflect/consistent_bindings_simple.hlsl index c63c5fecd0..021eb82877 100644 --- a/tools/clang/test/HLSLFileCheck/d3dreflect/consistent_bindings_simple.hlsl +++ b/tools/clang/test/HLSLFileCheck/d3dreflect/consistent_bindings_simple.hlsl @@ -1,6 +1,6 @@ -// RUN: %dxc -T lib_6_3 -auto-binding-space 0 -fhlsl-unused-resource-bindings=reserve %s | %D3DReflect %s | FileCheck %s -check-prefix=CHECK -// RUN: %dxc -T cs_6_3 -E mainA -auto-binding-space 0 -fhlsl-unused-resource-bindings=reserve %s | %D3DReflect %s | FileCheck %s -check-prefix=CHECK2 -// RUN: %dxc -T cs_6_3 -E mainB -auto-binding-space 0 -fhlsl-unused-resource-bindings=reserve %s | %D3DReflect %s | FileCheck %s -check-prefix=CHECK3 +// RUN: %dxc -T lib_6_3 -auto-binding-space 0 -fhlsl-unused-resource-bindings=reserve-all %s | %D3DReflect %s | FileCheck %s -check-prefix=CHECK +// RUN: %dxc -T cs_6_3 -E mainA -auto-binding-space 0 -fhlsl-unused-resource-bindings=reserve-all %s | %D3DReflect %s | FileCheck %s -check-prefix=CHECK2 +// RUN: %dxc -T cs_6_3 -E mainB -auto-binding-space 0 -fhlsl-unused-resource-bindings=reserve-all %s | %D3DReflect %s | FileCheck %s -check-prefix=CHECK3 RWByteAddressBuffer a; RWByteAddressBuffer b; diff --git a/tools/clang/tools/dxcompiler/dxcompilerobj.cpp b/tools/clang/tools/dxcompiler/dxcompilerobj.cpp index 6b956e0a04..49445f448c 100644 --- a/tools/clang/tools/dxcompiler/dxcompilerobj.cpp +++ b/tools/clang/tools/dxcompiler/dxcompilerobj.cpp @@ -1596,7 +1596,7 @@ class DxcCompiler : public IDxcCompiler3, compiler.getCodeGenOpts().HLSLLegacyResourceReservation = Opts.LegacyResourceReservation; compiler.getCodeGenOpts().HLSLConsistentBindings = - Opts.UnusedResources == hlsl::options::UnusedResourceBinding::Reserve; + Opts.UnusedResources == hlsl::options::UnusedResourceBinding::ReserveAll; compiler.getCodeGenOpts().HLSLDefines = defines; compiler.getCodeGenOpts().HLSLPreciseOutputs = Opts.PreciseOutputs; compiler.getCodeGenOpts().MainFileName = pMainFile; From 6d0531b0a2aaefadce028ce7bf21ebee6b3f4068 Mon Sep 17 00:00:00 2001 From: Nielsbishere Date: Thu, 13 Nov 2025 14:47:18 +0100 Subject: [PATCH 19/29] Removed ConsistentBindings from intermediate flags and moved it to DxilShaderFlags instead. This is because 'keep-all' from the other PR relies on this Enum to be serialized; because reflection needs to know if unused resources are preserved. --- include/dxc/DXIL/DxilConstants.h | 6 ++++++ include/dxc/DXIL/DxilModule.h | 10 ++++++---- include/dxc/DXIL/DxilShaderFlags.h | 13 ++++++++++++- include/dxc/HLSL/HLModule.h | 6 +++--- include/dxc/Support/HLSLOptions.h | 5 ++--- lib/DXIL/DxilModule.cpp | 11 +++++------ lib/DXIL/DxilShaderFlags.cpp | 3 ++- lib/DxcSupport/HLSLOptions.cpp | 4 ++-- lib/HLSL/DxilCondenseResources.cpp | 6 +++--- lib/HLSL/DxilGenerationPass.cpp | 3 ++- lib/HLSL/HLModule.cpp | 19 ++++++++----------- .../include/clang/Frontend/CodeGenOptions.h | 5 +++-- tools/clang/lib/CodeGen/CGHLSLMS.cpp | 2 +- .../clang/tools/dxcompiler/dxcompilerobj.cpp | 3 +-- 14 files changed, 56 insertions(+), 40 deletions(-) diff --git a/include/dxc/DXIL/DxilConstants.h b/include/dxc/DXIL/DxilConstants.h index d66f14a1e9..b6b18a67b8 100644 --- a/include/dxc/DXIL/DxilConstants.h +++ b/include/dxc/DXIL/DxilConstants.h @@ -15,6 +15,12 @@ namespace hlsl { +enum class UnusedResourceBinding : uint32_t { Strip, ReserveAll, Count }; + +static_assert(UnusedResourceBinding::Count <= UnusedResourceBinding(7), + "Only 3 bits are reserved for UnusedResourceBinding by HLOptions " + "and ShaderFlags"); + /* import hctdb_instrhelp */ diff --git a/include/dxc/DXIL/DxilModule.h b/include/dxc/DXIL/DxilModule.h index ee5bd28f3a..05626c1172 100644 --- a/include/dxc/DXIL/DxilModule.h +++ b/include/dxc/DXIL/DxilModule.h @@ -20,6 +20,7 @@ #include "dxc/DXIL/DxilSignature.h" #include "dxc/DXIL/DxilSubobject.h" #include "dxc/DXIL/DxilTypeSystem.h" +#include "dxc/DXIL/DxilConstants.h" #include #include @@ -287,8 +288,9 @@ 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 SetUnusedResourceBinding(UnusedResourceBinding unusedResourceBinding); + UnusedResourceBinding GetUnusedResourceBinding() const; void ClearIntermediateOptions(); // Hull and Domain shaders. @@ -347,8 +349,7 @@ class DxilModule { unsigned m_ActiveStreamMask = 0; enum IntermediateFlags : uint32_t { - LegacyResourceReservation = 1 << 0, - ConsistentBindings = 1 << 1 + LegacyResourceReservation = 1 << 0 }; llvm::LLVMContext &m_Ctx; @@ -386,6 +387,7 @@ class DxilModule { bool m_bUseMinPrecision = true; // use min precision by default; bool m_bAllResourcesBound = false; bool m_bResMayAlias = false; + UnusedResourceBinding m_unusedResourceBinding = UnusedResourceBinding::Strip; // properties from HLModule that should not make it to the final DXIL uint32_t m_IntermediateFlags = 0; diff --git a/include/dxc/DXIL/DxilShaderFlags.h b/include/dxc/DXIL/DxilShaderFlags.h index 7b065c63fa..44c1ddfdfb 100644 --- a/include/dxc/DXIL/DxilShaderFlags.h +++ b/include/dxc/DXIL/DxilShaderFlags.h @@ -16,6 +16,7 @@ namespace hlsl { class DxilModule; struct DxilFunctionProps; +enum class UnusedResourceBinding : uint32_t; } namespace llvm { @@ -219,6 +220,14 @@ class ShaderFlags { void SetRequiresGroup(bool flag) { m_bRequiresGroup = flag; } bool GetRequiresGroup() const { return m_bRequiresGroup; } + void SetUnusedResourceBinding(UnusedResourceBinding bindings) { + m_UnusedResourceBinding = unsigned(bindings); + } + + UnusedResourceBinding GetUnusedResourceBinding() { + return UnusedResourceBinding(m_UnusedResourceBinding); + } + private: // Bit: 0 unsigned @@ -359,7 +368,9 @@ class ShaderFlags { unsigned m_bRequiresGroup : 1; // SHADER_FEATURE_OPT_REQUIRES_GROUP // (OptFeatureInfo_RequiresGroup) - uint32_t m_align1 : 23; // align to 64 bit. + unsigned m_UnusedResourceBinding : 3; + + uint32_t m_align1 : 20; // align to 64 bit. }; } // namespace hlsl diff --git a/include/dxc/HLSL/HLModule.h b/include/dxc/HLSL/HLModule.h index 351366db6f..de851121fb 100644 --- a/include/dxc/HLSL/HLModule.h +++ b/include/dxc/HLSL/HLModule.h @@ -54,7 +54,7 @@ struct HLOptions { bDisableOptimizations(false), PackingStrategy(0), bUseMinPrecision(false), bDX9CompatMode(false), bFXCCompatMode(false), bLegacyResourceReservation(false), bForceZeroStoreLifetimes(false), - bConsistentBindings(false), unused(0) {} + bUnusedResourceBinding(0), unused(0) {} uint32_t GetHLOptionsRaw() const; void SetHLOptionsRaw(uint32_t data); unsigned bDefaultRowMajor : 1; @@ -70,8 +70,8 @@ struct HLOptions { unsigned bLegacyResourceReservation : 1; unsigned bForceZeroStoreLifetimes : 1; unsigned bResMayAlias : 1; - unsigned bConsistentBindings : 1; - unsigned unused : 19; + unsigned bUnusedResourceBinding : 3; + unsigned unused : 17; }; typedef std::unordered_mapGetUseMinPrecision()); flag.SetDisableOptimizations(M->GetDisableOptimization()); flag.SetAllResourcesBound(M->GetAllResourcesBound()); + flag.SetUnusedResourceBinding(M->GetUnusedResourceBinding()); bool hasDouble = false; // ddiv dfma drcp d2i d2u i2d u2d. diff --git a/lib/DxcSupport/HLSLOptions.cpp b/lib/DxcSupport/HLSLOptions.cpp index 1fbdca06d1..f46f70105d 100644 --- a/lib/DxcSupport/HLSLOptions.cpp +++ b/lib/DxcSupport/HLSLOptions.cpp @@ -870,9 +870,9 @@ int ReadDxcOpts(const OptTable *optionTable, unsigned flagsToInclude, Args.getLastArgValue(OPT_fhlsl_unused_resource_bindings_EQ, "strip"); if (UnusedResources == "strip") - opts.UnusedResources = UnusedResourceBinding::Strip; + opts.UnusedResourceBinding = UnusedResourceBinding::Strip; else if (UnusedResources == "reserve-all") - opts.UnusedResources = UnusedResourceBinding::ReserveAll; + opts.UnusedResourceBinding = UnusedResourceBinding::ReserveAll; else { errors << "Error: Invalid value for -fhlsl-unused-resource-bindings option " "specified (" diff --git a/lib/HLSL/DxilCondenseResources.cpp b/lib/HLSL/DxilCondenseResources.cpp index 824a9b4c4f..0c900f5e86 100644 --- a/lib/HLSL/DxilCondenseResources.cpp +++ b/lib/HLSL/DxilCondenseResources.cpp @@ -550,7 +550,7 @@ class DxilLowerCreateHandleForLib : public ModulePass { ResourceRegisterAllocator.GatherReservedRegisters(DM); // Remove unused resources. - if (!DM.GetConsistentBindings()) + if (DM.GetUnusedResourceBinding() == UnusedResourceBinding::Strip) bChanged |= DM.RemoveResourcesWithUnusedSymbols(); unsigned newResources = DM.GetCBuffers().size() + DM.GetUAVs().size() + @@ -562,7 +562,7 @@ class DxilLowerCreateHandleForLib : public ModulePass { { DxilValueCache *DVC = &getAnalysis(); bool bLocalChanged = LegalizeResources(M, DVC); - if (bLocalChanged && !DM.GetConsistentBindings()) { + if (bLocalChanged && DM.GetUnusedResourceBinding() == UnusedResourceBinding::Strip) { // Remove unused resources. bChanged |= DM.RemoveResourcesWithUnusedSymbols(); } @@ -571,7 +571,7 @@ class DxilLowerCreateHandleForLib : public ModulePass { bChanged |= ResourceRegisterAllocator.AllocateRegisters(DM); - if (DM.GetConsistentBindings()) + if (DM.GetUnusedResourceBinding() == UnusedResourceBinding::ReserveAll) bChanged |= DM.RemoveResourcesWithUnusedSymbols(); // Fill in top-level CBuffer variable usage bit diff --git a/lib/HLSL/DxilGenerationPass.cpp b/lib/HLSL/DxilGenerationPass.cpp index 1686482518..d08041c77b 100644 --- a/lib/HLSL/DxilGenerationPass.cpp +++ b/lib/HLSL/DxilGenerationPass.cpp @@ -155,7 +155,8 @@ 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); + M.SetUnusedResourceBinding( + UnusedResourceBinding(H.GetHLOptions().bUnusedResourceBinding)); // bool m_bDisableMathRefactoring; // bool m_bEnableDoublePrecision; // bool m_bEnableDoubleExtensions; diff --git a/lib/HLSL/HLModule.cpp b/lib/HLSL/HLModule.cpp index 41cafb971d..9da9b74dae 100644 --- a/lib/HLSL/HLModule.cpp +++ b/lib/HLSL/HLModule.cpp @@ -232,12 +232,12 @@ namespace { template bool RemoveResource(std::vector> &vec, GlobalVariable *pVariable, bool keepAllocated, - bool consistentBindings) { + UnusedResourceBinding unusedResourceBinding) { for (auto p = vec.begin(), e = vec.end(); p != e; ++p) { if ((*p)->GetGlobalSymbol() != pVariable) continue; - if ((keepAllocated && (*p)->IsAllocated()) || consistentBindings) { + if ((keepAllocated && (*p)->IsAllocated()) || unusedResourceBinding == UnusedResourceBinding::ReserveAll) { // Keep the resource, but it has no more symbol. (*p)->SetGlobalSymbol(UndefValue::get(pVariable->getType())); } else { @@ -264,22 +264,19 @@ 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; + UnusedResourceBinding unusedResourceBinding = + UnusedResourceBinding(GetHLOptions().bUnusedResourceBinding); // 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, consistentBindings)) + if (RemoveResource(m_CBuffers, GV, keepAllocated, unusedResourceBinding)) return; - if (RemoveResource(m_SRVs, GV, keepAllocated, consistentBindings)) + if (RemoveResource(m_SRVs, GV, keepAllocated, unusedResourceBinding)) return; - if (RemoveResource(m_UAVs, GV, keepAllocated, consistentBindings)) + if (RemoveResource(m_UAVs, GV, keepAllocated, unusedResourceBinding)) return; - if (RemoveResource(m_Samplers, GV, keepAllocated, consistentBindings)) + if (RemoveResource(m_Samplers, GV, keepAllocated, unusedResourceBinding)) 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 2fe3d418b9..b69cf75360 100644 --- a/tools/clang/include/clang/Frontend/CodeGenOptions.h +++ b/tools/clang/include/clang/Frontend/CodeGenOptions.h @@ -187,8 +187,9 @@ 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; + /// How to handle unused resource bindings. + hlsl::UnusedResourceBinding HLSLUnusedResourceBinding = + hlsl::UnusedResourceBinding::Strip; /// 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 18d3146250..093a4d0db2 100644 --- a/tools/clang/lib/CodeGen/CGHLSLMS.cpp +++ b/tools/clang/lib/CodeGen/CGHLSLMS.cpp @@ -400,7 +400,7 @@ CGMSHLSLRuntime::CGMSHLSLRuntime(CodeGenModule &CGM) opts.PackingStrategy = CGM.getCodeGenOpts().HLSLSignaturePackingStrategy; opts.bLegacyResourceReservation = CGM.getCodeGenOpts().HLSLLegacyResourceReservation; - opts.bConsistentBindings = CGM.getCodeGenOpts().HLSLConsistentBindings; + opts.bUnusedResourceBinding = unsigned(CGM.getCodeGenOpts().HLSLUnusedResourceBinding); opts.bForceZeroStoreLifetimes = CGM.getCodeGenOpts().HLSLForceZeroStoreLifetimes; diff --git a/tools/clang/tools/dxcompiler/dxcompilerobj.cpp b/tools/clang/tools/dxcompiler/dxcompilerobj.cpp index fde145bdf7..78e0a0cb33 100644 --- a/tools/clang/tools/dxcompiler/dxcompilerobj.cpp +++ b/tools/clang/tools/dxcompiler/dxcompilerobj.cpp @@ -1595,8 +1595,7 @@ class DxcCompiler : public IDxcCompiler3, compiler.getCodeGenOpts().HLSLAvoidControlFlow = Opts.AvoidFlowControl; compiler.getCodeGenOpts().HLSLLegacyResourceReservation = Opts.LegacyResourceReservation; - compiler.getCodeGenOpts().HLSLConsistentBindings = - Opts.UnusedResources == hlsl::options::UnusedResourceBinding::ReserveAll; + compiler.getCodeGenOpts().HLSLUnusedResourceBinding = Opts.UnusedResourceBinding; compiler.getCodeGenOpts().HLSLDefines = defines; compiler.getCodeGenOpts().HLSLPreciseOutputs = Opts.PreciseOutputs; compiler.getCodeGenOpts().MainFileName = pMainFile; From 75f9cc0afb54dadfbe8942ab9486b2cf1765baa3 Mon Sep 17 00:00:00 2001 From: Nielsbishere Date: Thu, 13 Nov 2025 16:03:53 +0100 Subject: [PATCH 20/29] Clang format --- include/dxc/DXIL/DxilModule.h | 5 +---- lib/DXIL/DxilModule.cpp | 3 ++- lib/HLSL/DxilCondenseResources.cpp | 3 ++- lib/HLSL/HLModule.cpp | 3 ++- tools/clang/lib/CodeGen/CGHLSLMS.cpp | 3 ++- tools/clang/tools/dxcompiler/dxcompilerobj.cpp | 3 ++- 6 files changed, 11 insertions(+), 9 deletions(-) diff --git a/include/dxc/DXIL/DxilModule.h b/include/dxc/DXIL/DxilModule.h index 05626c1172..0b6aff27d7 100644 --- a/include/dxc/DXIL/DxilModule.h +++ b/include/dxc/DXIL/DxilModule.h @@ -20,7 +20,6 @@ #include "dxc/DXIL/DxilSignature.h" #include "dxc/DXIL/DxilSubobject.h" #include "dxc/DXIL/DxilTypeSystem.h" -#include "dxc/DXIL/DxilConstants.h" #include #include @@ -348,9 +347,7 @@ class DxilModule { DXIL::PrimitiveTopology::Undefined; unsigned m_ActiveStreamMask = 0; - enum IntermediateFlags : uint32_t { - LegacyResourceReservation = 1 << 0 - }; + enum IntermediateFlags : uint32_t { LegacyResourceReservation = 1 << 0 }; llvm::LLVMContext &m_Ctx; llvm::Module *m_pModule = nullptr; diff --git a/lib/DXIL/DxilModule.cpp b/lib/DXIL/DxilModule.cpp index 58c9471661..162732fd89 100644 --- a/lib/DXIL/DxilModule.cpp +++ b/lib/DXIL/DxilModule.cpp @@ -586,7 +586,8 @@ bool DxilModule::GetLegacyResourceReservation() const { return (m_IntermediateFlags & LegacyResourceReservation) != 0; } -void DxilModule::SetUnusedResourceBinding(UnusedResourceBinding unusedResourceBinding) { +void DxilModule::SetUnusedResourceBinding( + UnusedResourceBinding unusedResourceBinding) { m_unusedResourceBinding = unusedResourceBinding; } diff --git a/lib/HLSL/DxilCondenseResources.cpp b/lib/HLSL/DxilCondenseResources.cpp index 0c900f5e86..0efee67614 100644 --- a/lib/HLSL/DxilCondenseResources.cpp +++ b/lib/HLSL/DxilCondenseResources.cpp @@ -562,7 +562,8 @@ class DxilLowerCreateHandleForLib : public ModulePass { { DxilValueCache *DVC = &getAnalysis(); bool bLocalChanged = LegalizeResources(M, DVC); - if (bLocalChanged && DM.GetUnusedResourceBinding() == UnusedResourceBinding::Strip) { + if (bLocalChanged && + DM.GetUnusedResourceBinding() == UnusedResourceBinding::Strip) { // Remove unused resources. bChanged |= DM.RemoveResourcesWithUnusedSymbols(); } diff --git a/lib/HLSL/HLModule.cpp b/lib/HLSL/HLModule.cpp index 9da9b74dae..af55bf616e 100644 --- a/lib/HLSL/HLModule.cpp +++ b/lib/HLSL/HLModule.cpp @@ -237,7 +237,8 @@ bool RemoveResource(std::vector> &vec, if ((*p)->GetGlobalSymbol() != pVariable) continue; - if ((keepAllocated && (*p)->IsAllocated()) || unusedResourceBinding == UnusedResourceBinding::ReserveAll) { + if ((keepAllocated && (*p)->IsAllocated()) || + unusedResourceBinding == UnusedResourceBinding::ReserveAll) { // Keep the resource, but it has no more symbol. (*p)->SetGlobalSymbol(UndefValue::get(pVariable->getType())); } else { diff --git a/tools/clang/lib/CodeGen/CGHLSLMS.cpp b/tools/clang/lib/CodeGen/CGHLSLMS.cpp index 093a4d0db2..c4edd003de 100644 --- a/tools/clang/lib/CodeGen/CGHLSLMS.cpp +++ b/tools/clang/lib/CodeGen/CGHLSLMS.cpp @@ -400,7 +400,8 @@ CGMSHLSLRuntime::CGMSHLSLRuntime(CodeGenModule &CGM) opts.PackingStrategy = CGM.getCodeGenOpts().HLSLSignaturePackingStrategy; opts.bLegacyResourceReservation = CGM.getCodeGenOpts().HLSLLegacyResourceReservation; - opts.bUnusedResourceBinding = unsigned(CGM.getCodeGenOpts().HLSLUnusedResourceBinding); + opts.bUnusedResourceBinding = + unsigned(CGM.getCodeGenOpts().HLSLUnusedResourceBinding); opts.bForceZeroStoreLifetimes = CGM.getCodeGenOpts().HLSLForceZeroStoreLifetimes; diff --git a/tools/clang/tools/dxcompiler/dxcompilerobj.cpp b/tools/clang/tools/dxcompiler/dxcompilerobj.cpp index 78e0a0cb33..f96cc7e6d3 100644 --- a/tools/clang/tools/dxcompiler/dxcompilerobj.cpp +++ b/tools/clang/tools/dxcompiler/dxcompilerobj.cpp @@ -1595,7 +1595,8 @@ class DxcCompiler : public IDxcCompiler3, compiler.getCodeGenOpts().HLSLAvoidControlFlow = Opts.AvoidFlowControl; compiler.getCodeGenOpts().HLSLLegacyResourceReservation = Opts.LegacyResourceReservation; - compiler.getCodeGenOpts().HLSLUnusedResourceBinding = Opts.UnusedResourceBinding; + compiler.getCodeGenOpts().HLSLUnusedResourceBinding = + Opts.UnusedResourceBinding; compiler.getCodeGenOpts().HLSLDefines = defines; compiler.getCodeGenOpts().HLSLPreciseOutputs = Opts.PreciseOutputs; compiler.getCodeGenOpts().MainFileName = pMainFile; From c2b24ed8ddd79004a720f1dba68830623071b51f Mon Sep 17 00:00:00 2001 From: NielsbishereAlt Date: Wed, 19 Nov 2025 19:20:14 +0100 Subject: [PATCH 21/29] Changed modif to Changed --- lib/DXIL/DxilModule.cpp | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/lib/DXIL/DxilModule.cpp b/lib/DXIL/DxilModule.cpp index 162732fd89..de2abad339 100644 --- a/lib/DXIL/DxilModule.cpp +++ b/lib/DXIL/DxilModule.cpp @@ -1032,7 +1032,7 @@ namespace { template static bool RemoveResourcesWithUnusedSymbolsHelper( std::vector> &vec) { - bool modif = false; + bool Changed = false; unsigned resID = 0; std::unordered_set eraseList; // Need in case of duplicate defs of lib resources @@ -1044,7 +1044,7 @@ static bool RemoveResourcesWithUnusedSymbolsHelper( p = vec.erase(c); if (GlobalVariable *GV = dyn_cast(symbol)) eraseList.insert(GV); - modif = true; + Changed = true; continue; } if ((*c)->GetID() != resID) { @@ -1055,17 +1055,17 @@ static bool RemoveResourcesWithUnusedSymbolsHelper( for (auto gv : eraseList) { gv->eraseFromParent(); } - return modif; + return Changed; } } // namespace bool DxilModule::RemoveResourcesWithUnusedSymbols() { - bool modif = false; - modif |= RemoveResourcesWithUnusedSymbolsHelper(m_SRVs); - modif |= RemoveResourcesWithUnusedSymbolsHelper(m_UAVs); - modif |= RemoveResourcesWithUnusedSymbolsHelper(m_CBuffers); - modif |= RemoveResourcesWithUnusedSymbolsHelper(m_Samplers); - return modif; + bool Changed = false; + Changed |= RemoveResourcesWithUnusedSymbolsHelper(m_SRVs); + Changed |= RemoveResourcesWithUnusedSymbolsHelper(m_UAVs); + Changed |= RemoveResourcesWithUnusedSymbolsHelper(m_CBuffers); + Changed |= RemoveResourcesWithUnusedSymbolsHelper(m_Samplers); + return Changed; } namespace { From d018f9446a2e96c4cfff7bdd0d1caae06e8cab96 Mon Sep 17 00:00:00 2001 From: Nielsbishere Date: Wed, 19 Nov 2025 23:11:20 +0100 Subject: [PATCH 22/29] Removed leftover b in front of bUnusedResourceBinding, removed unused resource bindings from shader flags. Added to IntermediateFlags instead. Updated static assert and enum cast. --- include/dxc/DXIL/DxilConstants.h | 4 ++-- include/dxc/DXIL/DxilModule.h | 9 ++++++++- include/dxc/DXIL/DxilShaderFlags.h | 12 +----------- include/dxc/HLSL/HLModule.h | 4 ++-- lib/DXIL/DxilModule.cpp | 9 ++++++--- lib/DXIL/DxilShaderFlags.cpp | 3 +-- lib/HLSL/DxilGenerationPass.cpp | 2 +- lib/HLSL/HLModule.cpp | 2 +- tools/clang/lib/CodeGen/CGHLSLMS.cpp | 2 +- 9 files changed, 23 insertions(+), 24 deletions(-) diff --git a/include/dxc/DXIL/DxilConstants.h b/include/dxc/DXIL/DxilConstants.h index b6b18a67b8..36372319bc 100644 --- a/include/dxc/DXIL/DxilConstants.h +++ b/include/dxc/DXIL/DxilConstants.h @@ -17,9 +17,9 @@ namespace hlsl { enum class UnusedResourceBinding : uint32_t { Strip, ReserveAll, Count }; -static_assert(UnusedResourceBinding::Count <= UnusedResourceBinding(7), +static_assert(unsigned(UnusedResourceBinding::Count) <= 7u, "Only 3 bits are reserved for UnusedResourceBinding by HLOptions " - "and ShaderFlags"); + "and IntermediateFlags"); /* import hctdb_instrhelp diff --git a/include/dxc/DXIL/DxilModule.h b/include/dxc/DXIL/DxilModule.h index 0b6aff27d7..d1ffd1cbcb 100644 --- a/include/dxc/DXIL/DxilModule.h +++ b/include/dxc/DXIL/DxilModule.h @@ -290,6 +290,7 @@ class DxilModule { void SetUnusedResourceBinding(UnusedResourceBinding unusedResourceBinding); UnusedResourceBinding GetUnusedResourceBinding() const; + void ClearIntermediateOptions(); // Hull and Domain shaders. @@ -347,7 +348,13 @@ class DxilModule { DXIL::PrimitiveTopology::Undefined; unsigned m_ActiveStreamMask = 0; - enum IntermediateFlags : uint32_t { LegacyResourceReservation = 1 << 0 }; + enum IntermediateFlags : uint32_t { + + LegacyResourceReservation = 1 << 0, + UnusedResourceBindingMask = 7 << 1, //3 bit reserved for unused resource binding + + UnusedResourceBindingShift = 1 + }; llvm::LLVMContext &m_Ctx; llvm::Module *m_pModule = nullptr; diff --git a/include/dxc/DXIL/DxilShaderFlags.h b/include/dxc/DXIL/DxilShaderFlags.h index 44c1ddfdfb..1b330eee95 100644 --- a/include/dxc/DXIL/DxilShaderFlags.h +++ b/include/dxc/DXIL/DxilShaderFlags.h @@ -220,14 +220,6 @@ class ShaderFlags { void SetRequiresGroup(bool flag) { m_bRequiresGroup = flag; } bool GetRequiresGroup() const { return m_bRequiresGroup; } - void SetUnusedResourceBinding(UnusedResourceBinding bindings) { - m_UnusedResourceBinding = unsigned(bindings); - } - - UnusedResourceBinding GetUnusedResourceBinding() { - return UnusedResourceBinding(m_UnusedResourceBinding); - } - private: // Bit: 0 unsigned @@ -368,9 +360,7 @@ class ShaderFlags { unsigned m_bRequiresGroup : 1; // SHADER_FEATURE_OPT_REQUIRES_GROUP // (OptFeatureInfo_RequiresGroup) - unsigned m_UnusedResourceBinding : 3; - - uint32_t m_align1 : 20; // align to 64 bit. + uint32_t m_align1 : 23; // align to 64 bit. }; } // namespace hlsl diff --git a/include/dxc/HLSL/HLModule.h b/include/dxc/HLSL/HLModule.h index de851121fb..06f98d6259 100644 --- a/include/dxc/HLSL/HLModule.h +++ b/include/dxc/HLSL/HLModule.h @@ -54,7 +54,7 @@ struct HLOptions { bDisableOptimizations(false), PackingStrategy(0), bUseMinPrecision(false), bDX9CompatMode(false), bFXCCompatMode(false), bLegacyResourceReservation(false), bForceZeroStoreLifetimes(false), - bUnusedResourceBinding(0), unused(0) {} + UnusedResourceBinding(0), unused(0) {} uint32_t GetHLOptionsRaw() const; void SetHLOptionsRaw(uint32_t data); unsigned bDefaultRowMajor : 1; @@ -70,7 +70,7 @@ struct HLOptions { unsigned bLegacyResourceReservation : 1; unsigned bForceZeroStoreLifetimes : 1; unsigned bResMayAlias : 1; - unsigned bUnusedResourceBinding : 3; + unsigned UnusedResourceBinding : 3; unsigned unused : 17; }; diff --git a/lib/DXIL/DxilModule.cpp b/lib/DXIL/DxilModule.cpp index de2abad339..1e19489274 100644 --- a/lib/DXIL/DxilModule.cpp +++ b/lib/DXIL/DxilModule.cpp @@ -588,11 +588,15 @@ bool DxilModule::GetLegacyResourceReservation() const { void DxilModule::SetUnusedResourceBinding( UnusedResourceBinding unusedResourceBinding) { - m_unusedResourceBinding = unusedResourceBinding; + m_IntermediateFlags &= ~UnusedResourceBindingMask; + m_IntermediateFlags |= unsigned(unusedResourceBinding) + << UnusedResourceBindingShift; } UnusedResourceBinding DxilModule::GetUnusedResourceBinding() const { - return m_unusedResourceBinding; + return (UnusedResourceBinding)((m_IntermediateFlags & + UnusedResourceBindingMask) >> + UnusedResourceBindingShift); } void DxilModule::ClearIntermediateOptions() { m_IntermediateFlags = 0; } @@ -1547,7 +1551,6 @@ void DxilModule::LoadDxilMetadata() { m_bUseMinPrecision = !m_ShaderFlags.GetUseNativeLowPrecision(); m_bDisableOptimizations = m_ShaderFlags.GetDisableOptimizations(); m_bAllResourcesBound = m_ShaderFlags.GetAllResourcesBound(); - m_unusedResourceBinding = m_ShaderFlags.GetUnusedResourceBinding(); m_bResMayAlias = !m_ShaderFlags.GetResMayNotAlias(); } diff --git a/lib/DXIL/DxilShaderFlags.cpp b/lib/DXIL/DxilShaderFlags.cpp index 9fc096dc86..993038aaf1 100644 --- a/lib/DXIL/DxilShaderFlags.cpp +++ b/lib/DXIL/DxilShaderFlags.cpp @@ -47,7 +47,7 @@ ShaderFlags::ShaderFlags() m_bAdvancedTextureOps(false), m_bWriteableMSAATextures(false), m_bReserved(false), m_bSampleCmpGradientOrBias(false), m_bExtendedCommandInfo(false), m_bUsesDerivatives(false), - m_bRequiresGroup(false), m_UnusedResourceBinding(0), m_align1(0) { + m_bRequiresGroup(false), m_align1(0) { // Silence unused field warnings (void)m_align1; } @@ -412,7 +412,6 @@ ShaderFlags ShaderFlags::CollectShaderFlags(const Function *F, flag.SetUseNativeLowPrecision(!M->GetUseMinPrecision()); flag.SetDisableOptimizations(M->GetDisableOptimization()); flag.SetAllResourcesBound(M->GetAllResourcesBound()); - flag.SetUnusedResourceBinding(M->GetUnusedResourceBinding()); bool hasDouble = false; // ddiv dfma drcp d2i d2u i2d u2d. diff --git a/lib/HLSL/DxilGenerationPass.cpp b/lib/HLSL/DxilGenerationPass.cpp index d08041c77b..de0cbb3b3a 100644 --- a/lib/HLSL/DxilGenerationPass.cpp +++ b/lib/HLSL/DxilGenerationPass.cpp @@ -156,7 +156,7 @@ void InitDxilModuleFromHLModule(HLModule &H, DxilModule &M, bool HasDebugInfo) { M.SetDisableOptimization(H.GetHLOptions().bDisableOptimizations); M.SetLegacyResourceReservation(H.GetHLOptions().bLegacyResourceReservation); M.SetUnusedResourceBinding( - UnusedResourceBinding(H.GetHLOptions().bUnusedResourceBinding)); + UnusedResourceBinding(H.GetHLOptions().UnusedResourceBinding)); // bool m_bDisableMathRefactoring; // bool m_bEnableDoublePrecision; // bool m_bEnableDoubleExtensions; diff --git a/lib/HLSL/HLModule.cpp b/lib/HLSL/HLModule.cpp index af55bf616e..7d6dd7513a 100644 --- a/lib/HLSL/HLModule.cpp +++ b/lib/HLSL/HLModule.cpp @@ -266,7 +266,7 @@ void HLModule::RemoveGlobal(llvm::GlobalVariable *GV) { bool keepAllocated = GetHLOptions().bLegacyResourceReservation; UnusedResourceBinding unusedResourceBinding = - UnusedResourceBinding(GetHLOptions().bUnusedResourceBinding); + UnusedResourceBinding(GetHLOptions().UnusedResourceBinding); // This could be considerably faster - check variable type to see which // resource type this is rather than scanning all lists, and look for diff --git a/tools/clang/lib/CodeGen/CGHLSLMS.cpp b/tools/clang/lib/CodeGen/CGHLSLMS.cpp index c4edd003de..85e6cc3b6e 100644 --- a/tools/clang/lib/CodeGen/CGHLSLMS.cpp +++ b/tools/clang/lib/CodeGen/CGHLSLMS.cpp @@ -400,7 +400,7 @@ CGMSHLSLRuntime::CGMSHLSLRuntime(CodeGenModule &CGM) opts.PackingStrategy = CGM.getCodeGenOpts().HLSLSignaturePackingStrategy; opts.bLegacyResourceReservation = CGM.getCodeGenOpts().HLSLLegacyResourceReservation; - opts.bUnusedResourceBinding = + opts.UnusedResourceBinding = unsigned(CGM.getCodeGenOpts().HLSLUnusedResourceBinding); opts.bForceZeroStoreLifetimes = CGM.getCodeGenOpts().HLSLForceZeroStoreLifetimes; From c8efa7bac72bd7fdaad3a56727739ee4e35910ca Mon Sep 17 00:00:00 2001 From: Nielsbishere Date: Wed, 19 Nov 2025 23:29:16 +0100 Subject: [PATCH 23/29] RemoveResourcesWithUnusedSymbolsHelper is now the one properly skipping removal of registers for ReserveAll it only kicks in after allocate registers. --- include/dxc/DXIL/DxilModule.h | 2 +- lib/DXIL/DxilModule.cpp | 20 ++++++++++++++------ lib/HLSL/DxilCondenseResources.cpp | 9 +++------ 3 files changed, 18 insertions(+), 13 deletions(-) diff --git a/include/dxc/DXIL/DxilModule.h b/include/dxc/DXIL/DxilModule.h index d1ffd1cbcb..63d29f987c 100644 --- a/include/dxc/DXIL/DxilModule.h +++ b/include/dxc/DXIL/DxilModule.h @@ -112,7 +112,7 @@ class DxilModule { const std::vector> &GetUAVs() const; void RemoveUnusedResources(); - bool RemoveResourcesWithUnusedSymbols(); + bool RemoveResourcesWithUnusedSymbols(bool AfterAllocation = false); void RemoveFunction(llvm::Function *F); bool RenameResourcesWithPrefix(const std::string &prefix); diff --git a/lib/DXIL/DxilModule.cpp b/lib/DXIL/DxilModule.cpp index 1e19489274..2ec1ff97d5 100644 --- a/lib/DXIL/DxilModule.cpp +++ b/lib/DXIL/DxilModule.cpp @@ -1035,7 +1035,8 @@ void DxilModule::RemoveUnusedResources() { namespace { template static bool RemoveResourcesWithUnusedSymbolsHelper( - std::vector> &vec) { + std::vector> &vec, bool AfterAllocation, + UnusedResourceBinding UnusedBinding) { bool Changed = false; unsigned resID = 0; std::unordered_set @@ -1045,6 +1046,9 @@ static bool RemoveResourcesWithUnusedSymbolsHelper( Constant *symbol = (*c)->GetGlobalSymbol(); symbol->removeDeadConstantUsers(); if (symbol->user_empty()) { + if (!AfterAllocation && + UnusedBinding == UnusedResourceBinding::ReserveAll) + continue; p = vec.erase(c); if (GlobalVariable *GV = dyn_cast(symbol)) eraseList.insert(GV); @@ -1063,12 +1067,16 @@ static bool RemoveResourcesWithUnusedSymbolsHelper( } } // namespace -bool DxilModule::RemoveResourcesWithUnusedSymbols() { +bool DxilModule::RemoveResourcesWithUnusedSymbols(bool AfterAllocation) { bool Changed = false; - Changed |= RemoveResourcesWithUnusedSymbolsHelper(m_SRVs); - Changed |= RemoveResourcesWithUnusedSymbolsHelper(m_UAVs); - Changed |= RemoveResourcesWithUnusedSymbolsHelper(m_CBuffers); - Changed |= RemoveResourcesWithUnusedSymbolsHelper(m_Samplers); + Changed |= RemoveResourcesWithUnusedSymbolsHelper(m_SRVs, AfterAllocation, + GetUnusedResourceBinding()); + Changed |= RemoveResourcesWithUnusedSymbolsHelper(m_UAVs, AfterAllocation, + GetUnusedResourceBinding()); + Changed |= RemoveResourcesWithUnusedSymbolsHelper(m_CBuffers, AfterAllocation, + GetUnusedResourceBinding()); + Changed |= RemoveResourcesWithUnusedSymbolsHelper(m_Samplers, AfterAllocation, + GetUnusedResourceBinding()); return Changed; } diff --git a/lib/HLSL/DxilCondenseResources.cpp b/lib/HLSL/DxilCondenseResources.cpp index 0efee67614..28b461fd54 100644 --- a/lib/HLSL/DxilCondenseResources.cpp +++ b/lib/HLSL/DxilCondenseResources.cpp @@ -551,7 +551,7 @@ class DxilLowerCreateHandleForLib : public ModulePass { // Remove unused resources. if (DM.GetUnusedResourceBinding() == UnusedResourceBinding::Strip) - bChanged |= DM.RemoveResourcesWithUnusedSymbols(); + bChanged |= DM.RemoveResourcesWithUnusedSymbols(); unsigned newResources = DM.GetCBuffers().size() + DM.GetUAVs().size() + DM.GetSRVs().size() + DM.GetSamplers().size(); @@ -562,8 +562,7 @@ class DxilLowerCreateHandleForLib : public ModulePass { { DxilValueCache *DVC = &getAnalysis(); bool bLocalChanged = LegalizeResources(M, DVC); - if (bLocalChanged && - DM.GetUnusedResourceBinding() == UnusedResourceBinding::Strip) { + if (bLocalChanged) { // Remove unused resources. bChanged |= DM.RemoveResourcesWithUnusedSymbols(); } @@ -571,9 +570,7 @@ class DxilLowerCreateHandleForLib : public ModulePass { } bChanged |= ResourceRegisterAllocator.AllocateRegisters(DM); - - if (DM.GetUnusedResourceBinding() == UnusedResourceBinding::ReserveAll) - bChanged |= DM.RemoveResourcesWithUnusedSymbols(); + bChanged |= DM.RemoveResourcesWithUnusedSymbols(true); // Fill in top-level CBuffer variable usage bit UpdateCBufferUsage(); From 5c96b53f8b0857d9634e8684b28eec61236a04c7 Mon Sep 17 00:00:00 2001 From: Nielsbishere Date: Wed, 19 Nov 2025 23:46:40 +0100 Subject: [PATCH 24/29] Removed leftover code --- lib/HLSL/DxilCondenseResources.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/HLSL/DxilCondenseResources.cpp b/lib/HLSL/DxilCondenseResources.cpp index 28b461fd54..034b37e581 100644 --- a/lib/HLSL/DxilCondenseResources.cpp +++ b/lib/HLSL/DxilCondenseResources.cpp @@ -550,7 +550,6 @@ class DxilLowerCreateHandleForLib : public ModulePass { ResourceRegisterAllocator.GatherReservedRegisters(DM); // Remove unused resources. - if (DM.GetUnusedResourceBinding() == UnusedResourceBinding::Strip) bChanged |= DM.RemoveResourcesWithUnusedSymbols(); unsigned newResources = DM.GetCBuffers().size() + DM.GetUAVs().size() + From 77275a4ad3070b0695413a9db5112eab1fbd3a05 Mon Sep 17 00:00:00 2001 From: Nielsbishere Date: Wed, 19 Nov 2025 23:51:21 +0100 Subject: [PATCH 25/29] Clang format and remove leftover dxil shader flag enum class --- include/dxc/DXIL/DxilModule.h | 3 ++- include/dxc/DXIL/DxilShaderFlags.h | 1 - 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/include/dxc/DXIL/DxilModule.h b/include/dxc/DXIL/DxilModule.h index 63d29f987c..7b93188f96 100644 --- a/include/dxc/DXIL/DxilModule.h +++ b/include/dxc/DXIL/DxilModule.h @@ -351,7 +351,8 @@ class DxilModule { enum IntermediateFlags : uint32_t { LegacyResourceReservation = 1 << 0, - UnusedResourceBindingMask = 7 << 1, //3 bit reserved for unused resource binding + UnusedResourceBindingMask = + 7 << 1, // 3 bit reserved for unused resource binding UnusedResourceBindingShift = 1 }; diff --git a/include/dxc/DXIL/DxilShaderFlags.h b/include/dxc/DXIL/DxilShaderFlags.h index 1b330eee95..7b065c63fa 100644 --- a/include/dxc/DXIL/DxilShaderFlags.h +++ b/include/dxc/DXIL/DxilShaderFlags.h @@ -16,7 +16,6 @@ namespace hlsl { class DxilModule; struct DxilFunctionProps; -enum class UnusedResourceBinding : uint32_t; } namespace llvm { From ba1b43b8dee1d5e182168afd6da86fe6d86efc57 Mon Sep 17 00:00:00 2001 From: Nielsbishere Date: Fri, 21 Nov 2025 15:58:04 +0100 Subject: [PATCH 26/29] Moved legacyResourceReservation into UnusedResourceBinding::ReserveExplicit. Changed IntermediateFlags to m_UnusedResourceBindings. ClearIntermediateOptions->ResetUnusedResourceBinding. Left -flegacy-resource-reservation as is for now, though it routes to UnusedResourceBinding::ReserveExplicit internally. --- include/dxc/DXIL/DxilConstants.h | 8 ++++- include/dxc/DXIL/DxilModule.h | 18 ++---------- include/dxc/HLSL/HLModule.h | 4 +-- include/dxc/Support/HLSLOptions.h | 3 +- lib/DXIL/DxilModule.cpp | 29 +++++-------------- lib/DxcSupport/HLSLOptions.cpp | 13 +++++++-- lib/HLSL/DxilCondenseResources.cpp | 2 +- lib/HLSL/DxilGenerationPass.cpp | 1 - lib/HLSL/DxilPreparePasses.cpp | 2 +- lib/HLSL/HLModule.cpp | 16 ++++------ .../include/clang/Frontend/CodeGenOptions.h | 2 -- tools/clang/lib/CodeGen/CGHLSLMS.cpp | 6 ++-- .../clang/tools/dxcompiler/dxcompilerobj.cpp | 2 -- tools/clang/unittests/HLSL/OptimizerTest.cpp | 2 +- 14 files changed, 43 insertions(+), 65 deletions(-) diff --git a/include/dxc/DXIL/DxilConstants.h b/include/dxc/DXIL/DxilConstants.h index 36372319bc..441ed30977 100644 --- a/include/dxc/DXIL/DxilConstants.h +++ b/include/dxc/DXIL/DxilConstants.h @@ -15,7 +15,13 @@ namespace hlsl { -enum class UnusedResourceBinding : uint32_t { Strip, ReserveAll, Count }; +enum class UnusedResourceBinding : uint32_t { + Strip, + ReserveAll, + Reserved, + ReserveExplicit, + Count +}; static_assert(unsigned(UnusedResourceBinding::Count) <= 7u, "Only 3 bits are reserved for UnusedResourceBinding by HLOptions " diff --git a/include/dxc/DXIL/DxilModule.h b/include/dxc/DXIL/DxilModule.h index 7b93188f96..03989803c9 100644 --- a/include/dxc/DXIL/DxilModule.h +++ b/include/dxc/DXIL/DxilModule.h @@ -284,14 +284,10 @@ class DxilModule { void SetResMayAlias(bool resMayAlias); bool GetResMayAlias() const; - // Intermediate options that do not make it to DXIL - void SetLegacyResourceReservation(bool legacyResourceReservation); - bool GetLegacyResourceReservation() const; - void SetUnusedResourceBinding(UnusedResourceBinding unusedResourceBinding); UnusedResourceBinding GetUnusedResourceBinding() const; - void ClearIntermediateOptions(); + void ResetUnusedResourceBinding(); // Hull and Domain shaders. unsigned GetInputControlPointCount() const; @@ -348,15 +344,6 @@ class DxilModule { DXIL::PrimitiveTopology::Undefined; unsigned m_ActiveStreamMask = 0; - enum IntermediateFlags : uint32_t { - - LegacyResourceReservation = 1 << 0, - UnusedResourceBindingMask = - 7 << 1, // 3 bit reserved for unused resource binding - - UnusedResourceBindingShift = 1 - }; - llvm::LLVMContext &m_Ctx; llvm::Module *m_pModule = nullptr; llvm::Function *m_pEntryFunc = nullptr; @@ -392,10 +379,9 @@ class DxilModule { bool m_bUseMinPrecision = true; // use min precision by default; bool m_bAllResourcesBound = false; bool m_bResMayAlias = false; - UnusedResourceBinding m_unusedResourceBinding = UnusedResourceBinding::Strip; // properties from HLModule that should not make it to the final DXIL - uint32_t m_IntermediateFlags = 0; + UnusedResourceBinding m_UnusedResourceBinding = UnusedResourceBinding::Strip; uint32_t m_AutoBindingSpace = UINT_MAX; std::unique_ptr m_pSubobjects; diff --git a/include/dxc/HLSL/HLModule.h b/include/dxc/HLSL/HLModule.h index 06f98d6259..473771d065 100644 --- a/include/dxc/HLSL/HLModule.h +++ b/include/dxc/HLSL/HLModule.h @@ -53,7 +53,7 @@ struct HLOptions { : bDefaultRowMajor(false), bIEEEStrict(false), bAllResourcesBound(false), bDisableOptimizations(false), PackingStrategy(0), bUseMinPrecision(false), bDX9CompatMode(false), bFXCCompatMode(false), - bLegacyResourceReservation(false), bForceZeroStoreLifetimes(false), + reserved(0), bForceZeroStoreLifetimes(false), UnusedResourceBinding(0), unused(0) {} uint32_t GetHLOptionsRaw() const; void SetHLOptionsRaw(uint32_t data); @@ -67,7 +67,7 @@ struct HLOptions { unsigned bUseMinPrecision : 1; unsigned bDX9CompatMode : 1; unsigned bFXCCompatMode : 1; - unsigned bLegacyResourceReservation : 1; + unsigned reserved : 1; unsigned bForceZeroStoreLifetimes : 1; unsigned bResMayAlias : 1; unsigned UnusedResourceBinding : 3; diff --git a/include/dxc/Support/HLSLOptions.h b/include/dxc/Support/HLSLOptions.h index 3f518669c1..afee491fde 100644 --- a/include/dxc/Support/HLSLOptions.h +++ b/include/dxc/Support/HLSLOptions.h @@ -213,7 +213,6 @@ class DxcOpts { bool DisassembleByteOffset = false; // OPT_No bool DisaseembleHex = false; // OPT_Lx bool LegacyMacroExpansion = false; // OPT_flegacy_macro_expansion - bool LegacyResourceReservation = false; // OPT_flegacy_resource_reservation unsigned long AutoBindingSpace = UINT_MAX; // OPT_auto_binding_space bool ExportShadersOnly = false; // OPT_export_shaders_only bool ResMayAlias = false; // OPT_res_may_alias @@ -229,7 +228,7 @@ class DxcOpts { unsigned TimeTraceGranularity = 500; // OPT_ftime_trace_granularity_EQ bool VerifyDiagnostics = false; // OPT_verify UnusedResourceBinding UnusedResourceBinding = - UnusedResourceBinding::Strip; // OPT_fhlsl_unused_resource_bindings_EQ + UnusedResourceBinding::Strip; // OPT_fhlsl_unused_resource_bindings_EQ and OPT_flegacy_resource_reservation bool Verbose = false; // OPT_verbose // Optimization pass enables, disables and selects diff --git a/lib/DXIL/DxilModule.cpp b/lib/DXIL/DxilModule.cpp index 2ec1ff97d5..3ad54db65f 100644 --- a/lib/DXIL/DxilModule.cpp +++ b/lib/DXIL/DxilModule.cpp @@ -576,30 +576,15 @@ void DxilModule::SetResMayAlias(bool resMayAlias) { bool DxilModule::GetResMayAlias() const { return m_bResMayAlias; } -void DxilModule::SetLegacyResourceReservation(bool legacyResourceReservation) { - m_IntermediateFlags &= ~LegacyResourceReservation; - if (legacyResourceReservation) - m_IntermediateFlags |= LegacyResourceReservation; -} - -bool DxilModule::GetLegacyResourceReservation() const { - return (m_IntermediateFlags & LegacyResourceReservation) != 0; -} - -void DxilModule::SetUnusedResourceBinding( - UnusedResourceBinding unusedResourceBinding) { - m_IntermediateFlags &= ~UnusedResourceBindingMask; - m_IntermediateFlags |= unsigned(unusedResourceBinding) - << UnusedResourceBindingShift; +void DxilModule::SetUnusedResourceBinding(UnusedResourceBinding unusedResourceBinding) { + m_UnusedResourceBinding = unusedResourceBinding; } UnusedResourceBinding DxilModule::GetUnusedResourceBinding() const { - return (UnusedResourceBinding)((m_IntermediateFlags & - UnusedResourceBindingMask) >> - UnusedResourceBindingShift); + return m_UnusedResourceBinding; } -void DxilModule::ClearIntermediateOptions() { m_IntermediateFlags = 0; } +void DxilModule::ResetUnusedResourceBinding() { m_UnusedResourceBinding = UnusedResourceBinding::Strip; } unsigned DxilModule::GetInputControlPointCount() const { if (!(m_pSM->IsHS() || m_pSM->IsDS())) @@ -1431,7 +1416,7 @@ void DxilModule::EmitDxilMetadata() { m_pMDHelper->EmitDxilVersion(m_DxilMajor, m_DxilMinor); m_pMDHelper->EmitValidatorVersion(m_ValMajor, m_ValMinor); m_pMDHelper->EmitDxilShaderModel(m_pSM); - m_pMDHelper->EmitDxilIntermediateOptions(m_IntermediateFlags); + m_pMDHelper->EmitDxilIntermediateOptions(uint32_t(m_UnusedResourceBinding)); MDTuple *pMDProperties = nullptr; uint64_t flag = m_ShaderFlags.GetShaderFlagsRaw(); @@ -1526,7 +1511,9 @@ void DxilModule::LoadDxilMetadata() { m_pMDHelper->LoadValidatorVersion(m_ValMajor, m_ValMinor); const ShaderModel *loadedSM; m_pMDHelper->LoadDxilShaderModel(loadedSM); - m_pMDHelper->LoadDxilIntermediateOptions(m_IntermediateFlags); + uint32_t options; + m_pMDHelper->LoadDxilIntermediateOptions(options); + m_UnusedResourceBinding = UnusedResourceBinding(options); // This must be set before LoadDxilEntryProperties m_pMDHelper->SetShaderModel(loadedSM); diff --git a/lib/DxcSupport/HLSLOptions.cpp b/lib/DxcSupport/HLSLOptions.cpp index f46f70105d..5e2a2eefc4 100644 --- a/lib/DxcSupport/HLSLOptions.cpp +++ b/lib/DxcSupport/HLSLOptions.cpp @@ -843,8 +843,6 @@ int ReadDxcOpts(const OptTable *optionTable, unsigned flagsToInclude, opts.DisaseembleHex = Args.hasFlag(OPT_Lx, OPT_INVALID, false); opts.LegacyMacroExpansion = Args.hasFlag(OPT_flegacy_macro_expansion, OPT_INVALID, false); - opts.LegacyResourceReservation = - Args.hasFlag(OPT_flegacy_resource_reservation, OPT_INVALID, false); opts.ExportShadersOnly = Args.hasFlag(OPT_export_shaders_only, OPT_INVALID, false); opts.PrintBeforeAll = Args.hasFlag(OPT_print_before_all, OPT_INVALID, false); @@ -880,6 +878,17 @@ int ReadDxcOpts(const OptTable *optionTable, unsigned flagsToInclude, return 1; } + // TODO: Move to OPT_fhlsl_unused_resource_bindings_EQ + if(Args.hasFlag(OPT_flegacy_resource_reservation, OPT_INVALID, false)) { + + if(unsigned(opts.UnusedResourceBinding)) { + errors << "Error: Unused resource bindings can't be used at the same time as flegacy_resource_reservation"; + return 1; + } + + opts.UnusedResourceBinding = UnusedResourceBinding::ReserveExplicit; + } + opts.Verbose = Args.hasFlag(OPT_verbose, OPT_INVALID, false); if (Args.hasArg(OPT_ftime_trace_EQ)) opts.TimeTrace = Args.getLastArgValue(OPT_ftime_trace_EQ); diff --git a/lib/HLSL/DxilCondenseResources.cpp b/lib/HLSL/DxilCondenseResources.cpp index 034b37e581..d7522fe15e 100644 --- a/lib/HLSL/DxilCondenseResources.cpp +++ b/lib/HLSL/DxilCondenseResources.cpp @@ -242,7 +242,7 @@ class DxilResourceRegisterAllocator { // For backcompat with FXC, shader models 5.0 and below will not // auto-allocate resources at a register explicitely assigned to even an // unused resource. - if (DM.GetLegacyResourceReservation()) { + if (DM.GetUnusedResourceBinding() == UnusedResourceBinding::ReserveExplicit) { GatherReservedRegisters(DM.GetCBuffers(), m_reservedCBufferRegisters); GatherReservedRegisters(DM.GetSamplers(), m_reservedSamplerRegisters); GatherReservedRegisters(DM.GetUAVs(), m_reservedUAVRegisters); diff --git a/lib/HLSL/DxilGenerationPass.cpp b/lib/HLSL/DxilGenerationPass.cpp index de0cbb3b3a..867d2948d4 100644 --- a/lib/HLSL/DxilGenerationPass.cpp +++ b/lib/HLSL/DxilGenerationPass.cpp @@ -154,7 +154,6 @@ void InitDxilModuleFromHLModule(HLModule &H, DxilModule &M, bool HasDebugInfo) { // Shader properties. // bool m_bDisableOptimizations; M.SetDisableOptimization(H.GetHLOptions().bDisableOptimizations); - M.SetLegacyResourceReservation(H.GetHLOptions().bLegacyResourceReservation); M.SetUnusedResourceBinding( UnusedResourceBinding(H.GetHLOptions().UnusedResourceBinding)); // bool m_bDisableMathRefactoring; diff --git a/lib/HLSL/DxilPreparePasses.cpp b/lib/HLSL/DxilPreparePasses.cpp index 68da520984..1e5c0bf12e 100644 --- a/lib/HLSL/DxilPreparePasses.cpp +++ b/lib/HLSL/DxilPreparePasses.cpp @@ -1003,7 +1003,7 @@ class DxilFinalizeModule : public ModulePass { DM.UpgradeToMinValidatorVersion(); // Clear intermediate options that shouldn't be in the final DXIL - DM.ClearIntermediateOptions(); + DM.ResetUnusedResourceBinding(); // Remove unused AllocateRayQuery calls RemoveUnusedRayQuery(M); diff --git a/lib/HLSL/HLModule.cpp b/lib/HLSL/HLModule.cpp index 7d6dd7513a..5efa8de79a 100644 --- a/lib/HLSL/HLModule.cpp +++ b/lib/HLSL/HLModule.cpp @@ -231,8 +231,9 @@ void HLModule::RemoveFunction(llvm::Function *F) { namespace { template bool RemoveResource(std::vector> &vec, - GlobalVariable *pVariable, bool keepAllocated, + GlobalVariable *pVariable, UnusedResourceBinding unusedResourceBinding) { + bool keepAllocated = unusedResourceBinding == UnusedResourceBinding::ReserveExplicit; for (auto p = vec.begin(), e = vec.end(); p != e; ++p) { if ((*p)->GetGlobalSymbol() != pVariable) continue; @@ -260,24 +261,19 @@ bool RemoveResource(std::vector> &vec, void HLModule::RemoveGlobal(llvm::GlobalVariable *GV) { DXASSERT_NOMSG(GV != nullptr); - // With legacy resource reservation, we must keep unused resources around - // when they have a register allocation because they prevent that - // register range from being allocated to other resources. - bool keepAllocated = GetHLOptions().bLegacyResourceReservation; - UnusedResourceBinding unusedResourceBinding = UnusedResourceBinding(GetHLOptions().UnusedResourceBinding); // 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, unusedResourceBinding)) + if (RemoveResource(m_CBuffers, GV, unusedResourceBinding)) return; - if (RemoveResource(m_SRVs, GV, keepAllocated, unusedResourceBinding)) + if (RemoveResource(m_SRVs, GV, unusedResourceBinding)) return; - if (RemoveResource(m_UAVs, GV, keepAllocated, unusedResourceBinding)) + if (RemoveResource(m_UAVs, GV, unusedResourceBinding)) return; - if (RemoveResource(m_Samplers, GV, keepAllocated, unusedResourceBinding)) + if (RemoveResource(m_Samplers, GV, unusedResourceBinding)) 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 b69cf75360..5126b37023 100644 --- a/tools/clang/include/clang/Frontend/CodeGenOptions.h +++ b/tools/clang/include/clang/Frontend/CodeGenOptions.h @@ -185,8 +185,6 @@ class CodeGenOptions : public CodeGenOptionsBase { bool HLSLAllowPreserveValues = false; /// Whether we fail compilation if loop fails to unroll bool HLSLOnlyWarnOnUnrollFail = false; - /// Whether use legacy resource reservation. - bool HLSLLegacyResourceReservation = false; /// How to handle unused resource bindings. hlsl::UnusedResourceBinding HLSLUnusedResourceBinding = hlsl::UnusedResourceBinding::Strip; diff --git a/tools/clang/lib/CodeGen/CGHLSLMS.cpp b/tools/clang/lib/CodeGen/CGHLSLMS.cpp index 85e6cc3b6e..f31819e252 100644 --- a/tools/clang/lib/CodeGen/CGHLSLMS.cpp +++ b/tools/clang/lib/CodeGen/CGHLSLMS.cpp @@ -42,6 +42,7 @@ #include #include "dxc/DXIL/DxilCBuffer.h" +#include "dxc/DXIL/DxilConstants.h" #include "dxc/DXIL/DxilResourceProperties.h" #include "dxc/DxilRootSignature/DxilRootSignature.h" #include "dxc/HLSL/DxilExportMap.h" @@ -398,8 +399,6 @@ CGMSHLSLRuntime::CGMSHLSLRuntime(CodeGenModule &CGM) opts.bAllResourcesBound = CGM.getCodeGenOpts().HLSLAllResourcesBound; opts.bResMayAlias = CGM.getCodeGenOpts().HLSLResMayAlias; opts.PackingStrategy = CGM.getCodeGenOpts().HLSLSignaturePackingStrategy; - opts.bLegacyResourceReservation = - CGM.getCodeGenOpts().HLSLLegacyResourceReservation; opts.UnusedResourceBinding = unsigned(CGM.getCodeGenOpts().HLSLUnusedResourceBinding); opts.bForceZeroStoreLifetimes = @@ -3132,7 +3131,8 @@ void GetResourceDeclElemTypeAndRangeSize(CodeGenModule &CGM, HLModule &HL, rangeSize *= cast(arrayType)->getSize().getLimitedValue(); } else { - if (HL.GetHLOptions().bLegacyResourceReservation) { + if (HL.GetHLOptions().UnusedResourceBinding == + unsigned(UnusedResourceBinding::ReserveExplicit)) { DiagnosticsEngine &Diags = CGM.getDiags(); unsigned DiagID = Diags.getCustomDiagID( DiagnosticsEngine::Error, "unbounded resources are not supported " diff --git a/tools/clang/tools/dxcompiler/dxcompilerobj.cpp b/tools/clang/tools/dxcompiler/dxcompilerobj.cpp index f96cc7e6d3..17cd32595f 100644 --- a/tools/clang/tools/dxcompiler/dxcompilerobj.cpp +++ b/tools/clang/tools/dxcompiler/dxcompilerobj.cpp @@ -1593,8 +1593,6 @@ class DxcCompiler : public IDxcCompiler3, compiler.getCodeGenOpts().HLSLOverrideSemDefs = Opts.OverrideSemDefs; compiler.getCodeGenOpts().HLSLPreferControlFlow = Opts.PreferFlowControl; compiler.getCodeGenOpts().HLSLAvoidControlFlow = Opts.AvoidFlowControl; - compiler.getCodeGenOpts().HLSLLegacyResourceReservation = - Opts.LegacyResourceReservation; compiler.getCodeGenOpts().HLSLUnusedResourceBinding = Opts.UnusedResourceBinding; compiler.getCodeGenOpts().HLSLDefines = defines; diff --git a/tools/clang/unittests/HLSL/OptimizerTest.cpp b/tools/clang/unittests/HLSL/OptimizerTest.cpp index 1294a41434..49c3a79a1f 100644 --- a/tools/clang/unittests/HLSL/OptimizerTest.cpp +++ b/tools/clang/unittests/HLSL/OptimizerTest.cpp @@ -163,7 +163,7 @@ TEST_F(OptimizerTest, OptimizerWhenSlice3ThenOK) { } TEST_F(OptimizerTest, OptimizerWhenSliceWithIntermediateOptionsThenOK) { - // The program below working depends on the LegacyResourceReservation option + // The program below working depends on the -flegacy-resource-reservation option // being carried through to the resource register allocator, even though it is // not preserved in the final shader. LPCSTR SampleProgram = "Texture2D tex0 : register(t0);\r\n" From 2f482a9e256e1f61ba5b95cbe3f8733f10397fa3 Mon Sep 17 00:00:00 2001 From: Nielsbishere Date: Fri, 21 Nov 2025 16:02:46 +0100 Subject: [PATCH 27/29] Clang format --- include/dxc/HLSL/HLModule.h | 4 ++-- include/dxc/Support/HLSLOptions.h | 3 ++- lib/DXIL/DxilModule.cpp | 7 +++++-- lib/DxcSupport/HLSLOptions.cpp | 7 ++++--- lib/HLSL/DxilCondenseResources.cpp | 3 ++- lib/HLSL/HLModule.cpp | 3 ++- tools/clang/unittests/HLSL/OptimizerTest.cpp | 6 +++--- 7 files changed, 20 insertions(+), 13 deletions(-) diff --git a/include/dxc/HLSL/HLModule.h b/include/dxc/HLSL/HLModule.h index 473771d065..55437fdb7c 100644 --- a/include/dxc/HLSL/HLModule.h +++ b/include/dxc/HLSL/HLModule.h @@ -53,8 +53,8 @@ struct HLOptions { : bDefaultRowMajor(false), bIEEEStrict(false), bAllResourcesBound(false), bDisableOptimizations(false), PackingStrategy(0), bUseMinPrecision(false), bDX9CompatMode(false), bFXCCompatMode(false), - reserved(0), bForceZeroStoreLifetimes(false), - UnusedResourceBinding(0), unused(0) {} + reserved(0), bForceZeroStoreLifetimes(false), UnusedResourceBinding(0), + unused(0) {} uint32_t GetHLOptionsRaw() const; void SetHLOptionsRaw(uint32_t data); unsigned bDefaultRowMajor : 1; diff --git a/include/dxc/Support/HLSLOptions.h b/include/dxc/Support/HLSLOptions.h index afee491fde..2c23743ef3 100644 --- a/include/dxc/Support/HLSLOptions.h +++ b/include/dxc/Support/HLSLOptions.h @@ -228,7 +228,8 @@ class DxcOpts { unsigned TimeTraceGranularity = 500; // OPT_ftime_trace_granularity_EQ bool VerifyDiagnostics = false; // OPT_verify UnusedResourceBinding UnusedResourceBinding = - UnusedResourceBinding::Strip; // OPT_fhlsl_unused_resource_bindings_EQ and OPT_flegacy_resource_reservation + UnusedResourceBinding::Strip; // OPT_fhlsl_unused_resource_bindings_EQ and + // OPT_flegacy_resource_reservation bool Verbose = false; // OPT_verbose // Optimization pass enables, disables and selects diff --git a/lib/DXIL/DxilModule.cpp b/lib/DXIL/DxilModule.cpp index 3ad54db65f..bd22d3bb31 100644 --- a/lib/DXIL/DxilModule.cpp +++ b/lib/DXIL/DxilModule.cpp @@ -576,7 +576,8 @@ void DxilModule::SetResMayAlias(bool resMayAlias) { bool DxilModule::GetResMayAlias() const { return m_bResMayAlias; } -void DxilModule::SetUnusedResourceBinding(UnusedResourceBinding unusedResourceBinding) { +void DxilModule::SetUnusedResourceBinding( + UnusedResourceBinding unusedResourceBinding) { m_UnusedResourceBinding = unusedResourceBinding; } @@ -584,7 +585,9 @@ UnusedResourceBinding DxilModule::GetUnusedResourceBinding() const { return m_UnusedResourceBinding; } -void DxilModule::ResetUnusedResourceBinding() { m_UnusedResourceBinding = UnusedResourceBinding::Strip; } +void DxilModule::ResetUnusedResourceBinding() { + m_UnusedResourceBinding = UnusedResourceBinding::Strip; +} unsigned DxilModule::GetInputControlPointCount() const { if (!(m_pSM->IsHS() || m_pSM->IsDS())) diff --git a/lib/DxcSupport/HLSLOptions.cpp b/lib/DxcSupport/HLSLOptions.cpp index 5e2a2eefc4..fad05cfcb7 100644 --- a/lib/DxcSupport/HLSLOptions.cpp +++ b/lib/DxcSupport/HLSLOptions.cpp @@ -879,10 +879,11 @@ int ReadDxcOpts(const OptTable *optionTable, unsigned flagsToInclude, } // TODO: Move to OPT_fhlsl_unused_resource_bindings_EQ - if(Args.hasFlag(OPT_flegacy_resource_reservation, OPT_INVALID, false)) { + if (Args.hasFlag(OPT_flegacy_resource_reservation, OPT_INVALID, false)) { - if(unsigned(opts.UnusedResourceBinding)) { - errors << "Error: Unused resource bindings can't be used at the same time as flegacy_resource_reservation"; + if (unsigned(opts.UnusedResourceBinding)) { + errors << "Error: Unused resource bindings can't be used at the same " + "time as flegacy_resource_reservation"; return 1; } diff --git a/lib/HLSL/DxilCondenseResources.cpp b/lib/HLSL/DxilCondenseResources.cpp index d7522fe15e..a1c17b67d2 100644 --- a/lib/HLSL/DxilCondenseResources.cpp +++ b/lib/HLSL/DxilCondenseResources.cpp @@ -242,7 +242,8 @@ class DxilResourceRegisterAllocator { // For backcompat with FXC, shader models 5.0 and below will not // auto-allocate resources at a register explicitely assigned to even an // unused resource. - if (DM.GetUnusedResourceBinding() == UnusedResourceBinding::ReserveExplicit) { + if (DM.GetUnusedResourceBinding() == + UnusedResourceBinding::ReserveExplicit) { GatherReservedRegisters(DM.GetCBuffers(), m_reservedCBufferRegisters); GatherReservedRegisters(DM.GetSamplers(), m_reservedSamplerRegisters); GatherReservedRegisters(DM.GetUAVs(), m_reservedUAVRegisters); diff --git a/lib/HLSL/HLModule.cpp b/lib/HLSL/HLModule.cpp index 5efa8de79a..d91d8c5116 100644 --- a/lib/HLSL/HLModule.cpp +++ b/lib/HLSL/HLModule.cpp @@ -233,7 +233,8 @@ template bool RemoveResource(std::vector> &vec, GlobalVariable *pVariable, UnusedResourceBinding unusedResourceBinding) { - bool keepAllocated = unusedResourceBinding == UnusedResourceBinding::ReserveExplicit; + bool keepAllocated = + unusedResourceBinding == UnusedResourceBinding::ReserveExplicit; for (auto p = vec.begin(), e = vec.end(); p != e; ++p) { if ((*p)->GetGlobalSymbol() != pVariable) continue; diff --git a/tools/clang/unittests/HLSL/OptimizerTest.cpp b/tools/clang/unittests/HLSL/OptimizerTest.cpp index 49c3a79a1f..f6ff1b83fe 100644 --- a/tools/clang/unittests/HLSL/OptimizerTest.cpp +++ b/tools/clang/unittests/HLSL/OptimizerTest.cpp @@ -163,9 +163,9 @@ TEST_F(OptimizerTest, OptimizerWhenSlice3ThenOK) { } TEST_F(OptimizerTest, OptimizerWhenSliceWithIntermediateOptionsThenOK) { - // The program below working depends on the -flegacy-resource-reservation option - // being carried through to the resource register allocator, even though it is - // not preserved in the final shader. + // The program below working depends on the -flegacy-resource-reservation + // option being carried through to the resource register allocator, even + // though it is not preserved in the final shader. LPCSTR SampleProgram = "Texture2D tex0 : register(t0);\r\n" "Texture2D tex1;\r\n" // tex1 should get register t1 "float4 main() : SV_Target {\r\n" From 4492c448109358401a66bab2dd96fd4906ab3814 Mon Sep 17 00:00:00 2001 From: Nielsbishere Date: Fri, 21 Nov 2025 16:43:07 +0100 Subject: [PATCH 28/29] HLModule::RemoveGlobal now relies on DxilCondenseResources to remove resources. The problem before was that it never called RemoveResourcesWithUnusedSymbols on early exit. Also my earlier attempt tried to access *p while it should've accessed *c, since *p was already incremented. --- lib/DXIL/DxilModule.cpp | 14 +++++++------- lib/HLSL/DxilCondenseResources.cpp | 2 ++ lib/HLSL/HLModule.cpp | 30 ++++++------------------------ 3 files changed, 15 insertions(+), 31 deletions(-) diff --git a/lib/DXIL/DxilModule.cpp b/lib/DXIL/DxilModule.cpp index bd22d3bb31..c351c7ec16 100644 --- a/lib/DXIL/DxilModule.cpp +++ b/lib/DXIL/DxilModule.cpp @@ -1025,6 +1025,8 @@ template static bool RemoveResourcesWithUnusedSymbolsHelper( std::vector> &vec, bool AfterAllocation, UnusedResourceBinding UnusedBinding) { + bool KeepAllocated = + UnusedBinding == UnusedResourceBinding::ReserveExplicit; bool Changed = false; unsigned resID = 0; std::unordered_set @@ -1035,7 +1037,8 @@ static bool RemoveResourcesWithUnusedSymbolsHelper( symbol->removeDeadConstantUsers(); if (symbol->user_empty()) { if (!AfterAllocation && - UnusedBinding == UnusedResourceBinding::ReserveAll) + (UnusedBinding == UnusedResourceBinding::ReserveAll || + (KeepAllocated && (*c)->IsAllocated()))) continue; p = vec.erase(c); if (GlobalVariable *GV = dyn_cast(symbol)) @@ -1059,12 +1062,9 @@ bool DxilModule::RemoveResourcesWithUnusedSymbols(bool AfterAllocation) { bool Changed = false; Changed |= RemoveResourcesWithUnusedSymbolsHelper(m_SRVs, AfterAllocation, GetUnusedResourceBinding()); - Changed |= RemoveResourcesWithUnusedSymbolsHelper(m_UAVs, AfterAllocation, - GetUnusedResourceBinding()); - Changed |= RemoveResourcesWithUnusedSymbolsHelper(m_CBuffers, AfterAllocation, - GetUnusedResourceBinding()); - Changed |= RemoveResourcesWithUnusedSymbolsHelper(m_Samplers, AfterAllocation, - GetUnusedResourceBinding()); + Changed |= RemoveResourcesWithUnusedSymbolsHelper(m_UAVs, AfterAllocation, GetUnusedResourceBinding()); + Changed |= RemoveResourcesWithUnusedSymbolsHelper(m_CBuffers, AfterAllocation, GetUnusedResourceBinding()); + Changed |= RemoveResourcesWithUnusedSymbolsHelper(m_Samplers, AfterAllocation, GetUnusedResourceBinding()); return Changed; } diff --git a/lib/HLSL/DxilCondenseResources.cpp b/lib/HLSL/DxilCondenseResources.cpp index a1c17b67d2..8c5b6d2cd3 100644 --- a/lib/HLSL/DxilCondenseResources.cpp +++ b/lib/HLSL/DxilCondenseResources.cpp @@ -528,6 +528,8 @@ class DxilLowerCreateHandleForLib : public ModulePass { SetNonUniformIndexForDynamicResource(DM); } + bChanged |= DM.RemoveResourcesWithUnusedSymbols(); + unsigned numResources = 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 d91d8c5116..f729103fe9 100644 --- a/lib/HLSL/HLModule.cpp +++ b/lib/HLSL/HLModule.cpp @@ -231,27 +231,12 @@ void HLModule::RemoveFunction(llvm::Function *F) { namespace { template bool RemoveResource(std::vector> &vec, - GlobalVariable *pVariable, - UnusedResourceBinding unusedResourceBinding) { - bool keepAllocated = - unusedResourceBinding == UnusedResourceBinding::ReserveExplicit; + GlobalVariable *pVariable) { for (auto p = vec.begin(), e = vec.end(); p != e; ++p) { if ((*p)->GetGlobalSymbol() != pVariable) continue; - if ((keepAllocated && (*p)->IsAllocated()) || - unusedResourceBinding == UnusedResourceBinding::ReserveAll) { - // 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); - } - } + (*p)->SetGlobalSymbol(UndefValue::get(pVariable->getType())); return true; } @@ -262,19 +247,16 @@ bool RemoveResource(std::vector> &vec, void HLModule::RemoveGlobal(llvm::GlobalVariable *GV) { DXASSERT_NOMSG(GV != nullptr); - UnusedResourceBinding unusedResourceBinding = - UnusedResourceBinding(GetHLOptions().UnusedResourceBinding); - // 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, unusedResourceBinding)) + if (RemoveResource(m_CBuffers, GV)) return; - if (RemoveResource(m_SRVs, GV, unusedResourceBinding)) + if (RemoveResource(m_SRVs, GV)) return; - if (RemoveResource(m_UAVs, GV, unusedResourceBinding)) + if (RemoveResource(m_UAVs, GV)) return; - if (RemoveResource(m_Samplers, GV, unusedResourceBinding)) + if (RemoveResource(m_Samplers, GV)) return; // TODO: do m_TGSMVariables and m_StreamOutputs need maintenance? } From 118363fc046821511de50b0c3c5a946893dd72c2 Mon Sep 17 00:00:00 2001 From: Nielsbishere Date: Fri, 21 Nov 2025 16:44:05 +0100 Subject: [PATCH 29/29] Clang format --- lib/DXIL/DxilModule.cpp | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/lib/DXIL/DxilModule.cpp b/lib/DXIL/DxilModule.cpp index c351c7ec16..f2cfebdb82 100644 --- a/lib/DXIL/DxilModule.cpp +++ b/lib/DXIL/DxilModule.cpp @@ -1025,8 +1025,7 @@ template static bool RemoveResourcesWithUnusedSymbolsHelper( std::vector> &vec, bool AfterAllocation, UnusedResourceBinding UnusedBinding) { - bool KeepAllocated = - UnusedBinding == UnusedResourceBinding::ReserveExplicit; + bool KeepAllocated = UnusedBinding == UnusedResourceBinding::ReserveExplicit; bool Changed = false; unsigned resID = 0; std::unordered_set @@ -1062,9 +1061,12 @@ bool DxilModule::RemoveResourcesWithUnusedSymbols(bool AfterAllocation) { bool Changed = false; Changed |= RemoveResourcesWithUnusedSymbolsHelper(m_SRVs, AfterAllocation, GetUnusedResourceBinding()); - Changed |= RemoveResourcesWithUnusedSymbolsHelper(m_UAVs, AfterAllocation, GetUnusedResourceBinding()); - Changed |= RemoveResourcesWithUnusedSymbolsHelper(m_CBuffers, AfterAllocation, GetUnusedResourceBinding()); - Changed |= RemoveResourcesWithUnusedSymbolsHelper(m_Samplers, AfterAllocation, GetUnusedResourceBinding()); + Changed |= RemoveResourcesWithUnusedSymbolsHelper(m_UAVs, AfterAllocation, + GetUnusedResourceBinding()); + Changed |= RemoveResourcesWithUnusedSymbolsHelper(m_CBuffers, AfterAllocation, + GetUnusedResourceBinding()); + Changed |= RemoveResourcesWithUnusedSymbolsHelper(m_Samplers, AfterAllocation, + GetUnusedResourceBinding()); return Changed; }