Skip to content

Commit

Permalink
Fix debug info nondeterminism in SROAGlobalsAndAllocas from staticGVs (
Browse files Browse the repository at this point in the history
…#5259)

staticGVs is used to track which globals are static and don't need new
debug info added at a certain point. Some global values are erased
during iteration of the work list, including globals that could be in
this set. This could lead to stale pointers in the set, which could
alias new global values from SROA, nondeterministically causing missing
debug info.

This change removes globals from the set (if present) in each path where
they are deleted. It also skips adding globals to the set (and
MergeGepUse) that have no users in the first place, since these will
just be deleted and have to be removed from the set later.

Fixes #5196.

---------

Co-authored-by: Greg Roth <[email protected]>
  • Loading branch information
tex3d and pow2clk authored Sep 13, 2023
1 parent b6bfe33 commit f80b8d6
Show file tree
Hide file tree
Showing 2 changed files with 739 additions and 4 deletions.
14 changes: 10 additions & 4 deletions lib/Transforms/Scalar/ScalarReplAggregatesHLSL.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1714,10 +1714,12 @@ bool SROAGlobalAndAllocas(HLModule &HLM, bool bHasDbgInfo) {
sz0 = getNestedLevelInStruct(a0ty);
sz1 = getNestedLevelInStruct(a1ty);
}
// If sizes are equal, tiebreak with alphabetically lesser at higher priority
return sz0 < sz1 || (sz0 == sz1 && isa<GlobalVariable>(a0) &&
isa<GlobalVariable>(a1) &&
a0->getName() > a1->getName());
// If sizes are equal, and the new value is a GV,
// replace the existing node if it isn't GV or comes later alphabetically
// Thus, entries are sorted by size, global variableness, and then name
return sz0 < sz1 || (sz0 == sz1 && isa<GlobalVariable>(a1) &&
(!isa<GlobalVariable>(a0) ||
a0->getName() > a1->getName()));
};

std::priority_queue<Value *, std::vector<Value *>,
Expand All @@ -1729,6 +1731,8 @@ bool SROAGlobalAndAllocas(HLModule &HLM, bool bHasDbgInfo) {

DenseMap<GlobalVariable *, GVDbgOffset> GVDbgOffsetMap;
for (GlobalVariable &GV : M.globals()) {
if (GV.user_empty())
continue;
if (dxilutil::IsStaticGlobal(&GV) || dxilutil::IsSharedMemoryGlobal(&GV)) {
staticGVs.insert(&GV);
GVDbgOffset &dbgOffset = GVDbgOffsetMap[&GV];
Expand Down Expand Up @@ -1895,6 +1899,7 @@ bool SROAGlobalAndAllocas(HLModule &HLM, bool bHasDbgInfo) {
// Handle dead GVs trivially. These can be formed by RAUWing one GV
// with another, leaving the original in the worklist
if (GV->use_empty()) {
staticGVs.remove(GV);
GV->eraseFromParent();
Changed = true;
continue;
Expand Down Expand Up @@ -1945,6 +1950,7 @@ bool SROAGlobalAndAllocas(HLModule &HLM, bool bHasDbgInfo) {
// Remove GV when it is replaced by NewEltGV and is not a base GV.
GV->removeDeadConstantUsers();
GV->eraseFromParent();
staticGVs.remove(GV);
}
GV = NewEltGV;
}
Expand Down
Loading

0 comments on commit f80b8d6

Please sign in to comment.