Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions docs/ReleaseNotes.md
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,9 @@ line upon naming the release. Refer to previous for appropriate section names.
- Fixed members following a merged bitfield reading the wrong value when a
struct is flat-converted
[#8526](https://github.com/microsoft/DirectXShaderCompiler/issues/8526).
- Fixed `-Zi` debug info for static globals used in inlined functions pointing
at the wrong subprogram, which the LLVM verifier rejects
[#8592](https://github.com/microsoft/DirectXShaderCompiler/issues/8592).

### Upcoming Preview Release

Expand Down
13 changes: 8 additions & 5 deletions lib/HLSL/DxilDeleteRedundantDebugValues.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -56,8 +56,11 @@ bool DxilDeleteRedundantDebugValues::runOnModule(Module &M) {
bool Changed = false;

std::unordered_set<DILocalScope *> SeenScopes;
typedef std::pair<DILocalVariable *, DIExpression *> VarPair;
SmallDenseMap<VarPair, DbgValueInst *> SeenVar;
// A variable is identified by its inlinedAt as well: each inlined instance
// of a scope has its own values for the variables in it.
typedef std::pair<std::pair<DILocalVariable *, DIExpression *>, DILocation *>
VarKey;
SmallDenseMap<VarKey, DbgValueInst *> SeenVar;

for (Function &F : M) {
SeenScopes.clear();
Expand Down Expand Up @@ -102,21 +105,21 @@ bool DxilDeleteRedundantDebugValues::runOnModule(Module &M) {
continue;
DILocalVariable *Var = DI->getVariable();
DIExpression *Expr = DI->getExpression();
VarPair Pair = VarPair(Var, Expr);
VarKey Key = VarKey({Var, Expr}, DI->getDebugLoc().getInlinedAt());

if (!SeenScopes.count(Var->getScope())) {
Changed = true;
DI->eraseFromParent();
continue;
}

auto findIt = SeenVar.find(Pair);
auto findIt = SeenVar.find(Key);
if (findIt != SeenVar.end()) {
findIt->second->eraseFromParent();
findIt->second = DI;
Changed = true;
} else {
SeenVar[Pair] = DI;
SeenVar[Key] = DI;
}
}
}
Expand Down
33 changes: 22 additions & 11 deletions lib/Transforms/Scalar/ScalarReplAggregatesHLSL.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -6537,11 +6537,14 @@ struct GVDebugInfoPatchCache {
return Result;
}

// Collect DISubprograms from a DILocation's inlined-at chain.
void CollectSubprograms(DILocation *Loc, SetVector<DISubprogram *> &Set) {
// Collect DISubprograms from a DILocation's inlined-at chain, together with
// the location each one is inlined at (null for the function itself).
void
CollectSubprograms(DILocation *Loc,
SetVector<std::pair<DISubprogram *, DILocation *>> &Set) {
while (Loc) {
if (DISubprogram *SP = GetSubprogramForLoc(Loc))
Set.insert(SP);
Set.insert({SP, Loc->getInlinedAt()});
Loc = Loc->getInlinedAt();
}
}
Expand Down Expand Up @@ -6786,22 +6789,30 @@ static void PatchDebugInfo(GVDebugInfoPatchCache &Cache,

// Only collect subprograms relevant to this GV to avoid creating
// O(subprograms × globals) debug instructions.
SetVector<DISubprogram *> Subprograms;
SetVector<std::pair<DISubprogram *, DILocation *>> Subprograms;

if (DISubprogram *SP = Cache.GetFuncSubprogram(F, DbgFinder))
Subprograms.insert(SP);
Subprograms.insert({SP, nullptr});

for (User *U : AI->users()) {
if (Instruction *I = dyn_cast<Instruction>(U))
Cache.CollectSubprograms(I->getDebugLoc(), Subprograms);
}

for (DISubprogram *Subprogram : Subprograms) {
DIScope *Scope = Subprogram;
DebugLoc Loc = DebugLoc::get(DGV->getLine(), 0, Scope);

DILocalVariable *ConvertedLocalVar = DIB.createLocalVariable(
Tag, Scope, Name, DGV->getFile(), DGV->getLine(), Ty);
// A subprogram inlined at several call sites shares one variable, but each
// inlined instance needs its own declare: its location must carry the
// inlinedAt chain, or it points at a subprogram other than F.
DenseMap<DISubprogram *, DILocalVariable *> LocalVars;
for (auto &SubprogramAndInlinedAt : Subprograms) {
DIScope *Scope = SubprogramAndInlinedAt.first;
DebugLoc Loc =
DebugLoc::get(DGV->getLine(), 0, Scope, SubprogramAndInlinedAt.second);
Comment on lines +6808 to +6809

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added a Bug Fixes entry under Upcoming Release in cf6a7fd.


DILocalVariable *&ConvertedLocalVar =
LocalVars[SubprogramAndInlinedAt.first];
if (!ConvertedLocalVar)
ConvertedLocalVar = DIB.createLocalVariable(
Tag, Scope, Name, DGV->getFile(), DGV->getLine(), Ty);
Comment on lines +6811 to +6815

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Confirmed: with two call sites only one inlined instance kept its dbg.value, at both -O3 and -Od. Not a regression from this PR (before it, the second call site had no declare at all), but cheap to fix here: cf6a7fd includes the inlinedAt in both LdStHasDebugValue() and DxilDeleteRedundantDebugValues, and the test now has a second call site. It fails without either of the two changes.


DIExpression *Expr = nullptr;
if (IsFragment)
Expand Down
17 changes: 10 additions & 7 deletions lib/Transforms/Utils/Local.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -972,18 +972,21 @@ unsigned llvm::getOrEnforceKnownAlignment(Value *V, unsigned PrefAlign,
/// Dbg Intrinsic utilities
///

/// See if there is a dbg.value intrinsic for DIVar before I.
static bool LdStHasDebugValue(const DILocalVariable *DIVar, Instruction *I) {
/// See if there is a dbg.value intrinsic for DDI's variable before I.
static bool LdStHasDebugValue(const DbgDeclareInst *DDI, Instruction *I) {
// Since we can't guarantee that the original dbg.declare instrinsic
// is removed by LowerDbgDeclare(), we need to make sure that we are
// not inserting the same dbg.value intrinsic over and over.
llvm::BasicBlock::InstListType::iterator PrevI(I);
if (PrevI != I->getParent()->getInstList().begin()) {
--PrevI;
if (DbgValueInst *DVI = dyn_cast<DbgValueInst>(PrevI))
if (DVI->getValue() == I->getOperand(0) &&
DVI->getOffset() == 0 &&
DVI->getVariable() == DIVar)
if (DVI->getValue() == I->getOperand(0) && DVI->getOffset() == 0 &&
DVI->getVariable() == DDI->getVariable() &&
// HLSL Change - Declares of one variable in different inlined
// instances only differ in their inlinedAt.
DVI->getDebugLoc().getInlinedAt() ==
DDI->getDebugLoc().getInlinedAt())
return true;
}
return false;
Expand All @@ -997,7 +1000,7 @@ bool llvm::ConvertDebugDeclareToDebugValue(DbgDeclareInst *DDI,
auto *DIExpr = DDI->getExpression();
assert(DIVar && "Missing variable");

if (LdStHasDebugValue(DIVar, SI))
if (LdStHasDebugValue(DDI, SI))
return true;

// If an argument is zero extended then use argument directly. The ZExt
Expand All @@ -1024,7 +1027,7 @@ bool llvm::ConvertDebugDeclareToDebugValue(DbgDeclareInst *DDI,
auto *DIExpr = DDI->getExpression();
assert(DIVar && "Missing variable");

if (LdStHasDebugValue(DIVar, LI))
if (LdStHasDebugValue(DDI, LI))
return true;

Builder.insertDbgValueIntrinsic(LI->getOperand(0), 0, DIVar, DIExpr,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
// RUN: %dxc -E main -T cs_6_6 -Zi %s | FileCheck %s

// Check that the debug info created for a localized global variable in an
// inlined scope carries the inlinedAt chain back to the entry point, instead of
// pointing at the inlined subprogram from inside main, and that every inlined
// instance keeps its own value.

// CHECK: @main

// The sampler handle is annotated at the location of one of the declares of
// the localized kSampler, all of which are inlined into main.
// CHECK: call %dx.types.Handle @dx.op.annotateHandle(i32 216, %dx.types.Handle %{{.+}}, %dx.types.ResourceProperties { i32 14, i32 0 }), !dbg ![[annot_loc:[0-9]+]]

// Both inlined instances of helper share one variable, but each gets a value.
// CHECK: call void @llvm.dbg.value(metadata i32 %{{.+}}, i64 0, metadata ![[helper_var:[0-9]+]], metadata !{{[0-9]+}}), !dbg ![[helper_loc0:[0-9]+]] ; var:"global.g_index" !DIExpression() func:"helper"
// CHECK: call void @llvm.dbg.value(metadata i32 %{{.+}}, i64 0, metadata ![[helper_var]], metadata !{{[0-9]+}}), !dbg ![[helper_loc1:[0-9]+]] ; var:"global.g_index" !DIExpression() func:"helper"

// CHECK-DAG: ![[main_scope:[0-9]+]] = !DISubprogram(name: "main"
// CHECK-DAG: ![[helper_scope:[0-9]+]] = !DISubprogram(name: "helper"

// CHECK-DAG: ![[helper_var]] = !DILocalVariable(tag: DW_TAG_arg_variable, name: "global.g_index", arg: 0, scope: ![[helper_scope]],
// CHECK-DAG: ![[helper_loc0]] = !DILocation(line: [[@LINE+8]], scope: ![[helper_scope]], inlinedAt: ![[call_loc0:[0-9]+]])
// CHECK-DAG: ![[helper_loc1]] = !DILocation(line: [[@LINE+7]], scope: ![[helper_scope]], inlinedAt: ![[call_loc1:[0-9]+]])
// CHECK-DAG: ![[annot_loc]] = !DILocation(line: [[@LINE+5]], scope: !{{[0-9]+}}, inlinedAt: !{{[0-9]+}})
// CHECK-DAG: ![[call_loc0]] = distinct !DILocation(line: [[@LINE+15]], column: {{[0-9]+}}, scope: ![[main_scope]])
// CHECK-DAG: ![[call_loc1]] = distinct !DILocation(line: [[@LINE+14]], column: {{[0-9]+}}, scope: ![[main_scope]])

SamplerState getSampler() { return SamplerDescriptorHeap[6]; }
static const SamplerState kSampler = getSampler();
static uint g_index;

float4 helper(float uv) {
Texture2D<float4> tex = ResourceDescriptorHeap[g_index];
return tex.SampleLevel(kSampler, uv, 0);
}

[numthreads(1, 1, 1)] void main(uint id : SV_GroupIndex) {
g_index = id;
RWBuffer<float4> output = ResourceDescriptorHeap[2];
output[0] = helper(0.5) + helper(0.25);
}
Loading