Skip to content

[RemoveDIs][DebugInfo] Handle DPVAssigns in Assignment Tracking excluding lowering #78982

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Merged
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
280 changes: 153 additions & 127 deletions llvm/lib/CodeGen/AssignmentTrackingAnalysis.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -829,6 +829,13 @@ class MemLocFragmentFill {
void process(BasicBlock &BB, VarFragMap &LiveSet) {
BBInsertBeforeMap[&BB].clear();
for (auto &I : BB) {
for (auto &DPV : I.getDbgValueRange()) {
if (const auto *Locs = FnVarLocs->getWedge(&DPV)) {
for (const VarLocInfo &Loc : *Locs) {
addDef(Loc, &DPV, *I.getParent(), LiveSet);
}
}
}
if (const auto *Locs = FnVarLocs->getWedge(&I)) {
for (const VarLocInfo &Loc : *Locs) {
addDef(Loc, &I, *I.getParent(), LiveSet);
Expand Down Expand Up @@ -2487,73 +2494,78 @@ removeRedundantDbgLocsUsingBackwardScan(const BasicBlock *BB,
VariableDefinedBytes.clear();
}

// Get the location defs that start just before this instruction.
const auto *Locs = FnVarLocs.getWedge(&I);
if (!Locs)
continue;
auto HandleLocsForWedge = [&](auto *WedgePosition) {
// Get the location defs that start just before this instruction.
const auto *Locs = FnVarLocs.getWedge(WedgePosition);
if (!Locs)
return;

NumWedgesScanned++;
bool ChangedThisWedge = false;
// The new pruned set of defs, reversed because we're scanning backwards.
SmallVector<VarLocInfo> NewDefsReversed;

// Iterate over the existing defs in reverse.
for (auto RIt = Locs->rbegin(), REnd = Locs->rend(); RIt != REnd; ++RIt) {
NumDefsScanned++;
DebugAggregate Aggr =
getAggregate(FnVarLocs.getVariable(RIt->VariableID));
uint64_t SizeInBits = Aggr.first->getSizeInBits().value_or(0);
uint64_t SizeInBytes = divideCeil(SizeInBits, 8);

// Cutoff for large variables to prevent expensive bitvector operations.
const uint64_t MaxSizeBytes = 2048;

if (SizeInBytes == 0 || SizeInBytes > MaxSizeBytes) {
// If the size is unknown (0) then keep this location def to be safe.
// Do the same for defs of large variables, which would be expensive
// to represent with a BitVector.
NewDefsReversed.push_back(*RIt);
continue;
}

NumWedgesScanned++;
bool ChangedThisWedge = false;
// The new pruned set of defs, reversed because we're scanning backwards.
SmallVector<VarLocInfo> NewDefsReversed;

// Iterate over the existing defs in reverse.
for (auto RIt = Locs->rbegin(), REnd = Locs->rend(); RIt != REnd; ++RIt) {
NumDefsScanned++;
DebugAggregate Aggr =
getAggregate(FnVarLocs.getVariable(RIt->VariableID));
uint64_t SizeInBits = Aggr.first->getSizeInBits().value_or(0);
uint64_t SizeInBytes = divideCeil(SizeInBits, 8);

// Cutoff for large variables to prevent expensive bitvector operations.
const uint64_t MaxSizeBytes = 2048;

if (SizeInBytes == 0 || SizeInBytes > MaxSizeBytes) {
// If the size is unknown (0) then keep this location def to be safe.
// Do the same for defs of large variables, which would be expensive
// to represent with a BitVector.
NewDefsReversed.push_back(*RIt);
continue;
}
// Only keep this location definition if it is not fully eclipsed by
// other definitions in this wedge that come after it

// Inert the bytes the location definition defines.
auto InsertResult =
VariableDefinedBytes.try_emplace(Aggr, BitVector(SizeInBytes));
bool FirstDefinition = InsertResult.second;
BitVector &DefinedBytes = InsertResult.first->second;

DIExpression::FragmentInfo Fragment =
RIt->Expr->getFragmentInfo().value_or(
DIExpression::FragmentInfo(SizeInBits, 0));
bool InvalidFragment = Fragment.endInBits() > SizeInBits;
uint64_t StartInBytes = Fragment.startInBits() / 8;
uint64_t EndInBytes = divideCeil(Fragment.endInBits(), 8);

// If this defines any previously undefined bytes, keep it.
if (FirstDefinition || InvalidFragment ||
DefinedBytes.find_first_unset_in(StartInBytes, EndInBytes) != -1) {
if (!InvalidFragment)
DefinedBytes.set(StartInBytes, EndInBytes);
NewDefsReversed.push_back(*RIt);
continue;
}

// Only keep this location definition if it is not fully eclipsed by
// other definitions in this wedge that come after it

// Inert the bytes the location definition defines.
auto InsertResult =
VariableDefinedBytes.try_emplace(Aggr, BitVector(SizeInBytes));
bool FirstDefinition = InsertResult.second;
BitVector &DefinedBytes = InsertResult.first->second;

DIExpression::FragmentInfo Fragment =
RIt->Expr->getFragmentInfo().value_or(
DIExpression::FragmentInfo(SizeInBits, 0));
bool InvalidFragment = Fragment.endInBits() > SizeInBits;
uint64_t StartInBytes = Fragment.startInBits() / 8;
uint64_t EndInBytes = divideCeil(Fragment.endInBits(), 8);

// If this defines any previously undefined bytes, keep it.
if (FirstDefinition || InvalidFragment ||
DefinedBytes.find_first_unset_in(StartInBytes, EndInBytes) != -1) {
if (!InvalidFragment)
DefinedBytes.set(StartInBytes, EndInBytes);
NewDefsReversed.push_back(*RIt);
continue;
// Redundant def found: throw it away. Since the wedge of defs is being
// rebuilt, doing nothing is the same as deleting an entry.
ChangedThisWedge = true;
NumDefsRemoved++;
}

// Redundant def found: throw it away. Since the wedge of defs is being
// rebuilt, doing nothing is the same as deleting an entry.
ChangedThisWedge = true;
NumDefsRemoved++;
}

// Un-reverse the defs and replace the wedge with the pruned version.
if (ChangedThisWedge) {
std::reverse(NewDefsReversed.begin(), NewDefsReversed.end());
FnVarLocs.setWedge(&I, std::move(NewDefsReversed));
NumWedgesChanged++;
Changed = true;
}
// Un-reverse the defs and replace the wedge with the pruned version.
if (ChangedThisWedge) {
std::reverse(NewDefsReversed.begin(), NewDefsReversed.end());
FnVarLocs.setWedge(WedgePosition, std::move(NewDefsReversed));
NumWedgesChanged++;
Changed = true;
}
};
HandleLocsForWedge(&I);
for (DPValue &DPV : reverse(I.getDbgValueRange()))
HandleLocsForWedge(&DPV);
}

return Changed;
Expand All @@ -2578,42 +2590,48 @@ removeRedundantDbgLocsUsingForwardScan(const BasicBlock *BB,
// instructions.
for (const Instruction &I : *BB) {
// Get the defs that come just before this instruction.
const auto *Locs = FnVarLocs.getWedge(&I);
if (!Locs)
continue;

NumWedgesScanned++;
bool ChangedThisWedge = false;
// The new pruned set of defs.
SmallVector<VarLocInfo> NewDefs;
auto HandleLocsForWedge = [&](auto *WedgePosition) {
const auto *Locs = FnVarLocs.getWedge(WedgePosition);
if (!Locs)
return;

NumWedgesScanned++;
bool ChangedThisWedge = false;
// The new pruned set of defs.
SmallVector<VarLocInfo> NewDefs;

// Iterate over the existing defs.
for (const VarLocInfo &Loc : *Locs) {
NumDefsScanned++;
DebugVariable Key(FnVarLocs.getVariable(Loc.VariableID).getVariable(),
std::nullopt, Loc.DL.getInlinedAt());
auto VMI = VariableMap.find(Key);

// Update the map if we found a new value/expression describing the
// variable, or if the variable wasn't mapped already.
if (VMI == VariableMap.end() || VMI->second.first != Loc.Values ||
VMI->second.second != Loc.Expr) {
VariableMap[Key] = {Loc.Values, Loc.Expr};
NewDefs.push_back(Loc);
continue;
}

// Iterate over the existing defs.
for (const VarLocInfo &Loc : *Locs) {
NumDefsScanned++;
DebugVariable Key(FnVarLocs.getVariable(Loc.VariableID).getVariable(),
std::nullopt, Loc.DL.getInlinedAt());
auto VMI = VariableMap.find(Key);

// Update the map if we found a new value/expression describing the
// variable, or if the variable wasn't mapped already.
if (VMI == VariableMap.end() || VMI->second.first != Loc.Values ||
VMI->second.second != Loc.Expr) {
VariableMap[Key] = {Loc.Values, Loc.Expr};
NewDefs.push_back(Loc);
continue;
// Did not insert this Loc, which is the same as removing it.
ChangedThisWedge = true;
NumDefsRemoved++;
}

// Did not insert this Loc, which is the same as removing it.
ChangedThisWedge = true;
NumDefsRemoved++;
}
// Replace the existing wedge with the pruned version.
if (ChangedThisWedge) {
FnVarLocs.setWedge(WedgePosition, std::move(NewDefs));
NumWedgesChanged++;
Changed = true;
}
};

// Replace the existing wedge with the pruned version.
if (ChangedThisWedge) {
FnVarLocs.setWedge(&I, std::move(NewDefs));
NumWedgesChanged++;
Changed = true;
}
for (DPValue &DPV : I.getDbgValueRange())
HandleLocsForWedge(&DPV);
HandleLocsForWedge(&I);
}

return Changed;
Expand Down Expand Up @@ -2660,41 +2678,46 @@ removeUndefDbgLocsFromEntryBlock(const BasicBlock *BB,
// instructions.
for (const Instruction &I : *BB) {
// Get the defs that come just before this instruction.
const auto *Locs = FnVarLocs.getWedge(&I);
if (!Locs)
continue;

NumWedgesScanned++;
bool ChangedThisWedge = false;
// The new pruned set of defs.
SmallVector<VarLocInfo> NewDefs;

// Iterate over the existing defs.
for (const VarLocInfo &Loc : *Locs) {
NumDefsScanned++;
DebugAggregate Aggr{FnVarLocs.getVariable(Loc.VariableID).getVariable(),
Loc.DL.getInlinedAt()};
DebugVariable Var = FnVarLocs.getVariable(Loc.VariableID);
auto HandleLocsForWedge = [&](auto *WedgePosition) {
const auto *Locs = FnVarLocs.getWedge(WedgePosition);
if (!Locs)
return;

NumWedgesScanned++;
bool ChangedThisWedge = false;
// The new pruned set of defs.
SmallVector<VarLocInfo> NewDefs;

// Iterate over the existing defs.
for (const VarLocInfo &Loc : *Locs) {
NumDefsScanned++;
DebugAggregate Aggr{FnVarLocs.getVariable(Loc.VariableID).getVariable(),
Loc.DL.getInlinedAt()};
DebugVariable Var = FnVarLocs.getVariable(Loc.VariableID);

// Remove undef entries that are encountered before any non-undef
// intrinsics from the entry block.
if (Loc.Values.isKillLocation(Loc.Expr) && !HasDefinedBits(Aggr, Var)) {
// Did not insert this Loc, which is the same as removing it.
NumDefsRemoved++;
ChangedThisWedge = true;
continue;
}

// Remove undef entries that are encountered before any non-undef
// intrinsics from the entry block.
if (Loc.Values.isKillLocation(Loc.Expr) && !HasDefinedBits(Aggr, Var)) {
// Did not insert this Loc, which is the same as removing it.
NumDefsRemoved++;
ChangedThisWedge = true;
continue;
DefineBits(Aggr, Var);
NewDefs.push_back(Loc);
}

DefineBits(Aggr, Var);
NewDefs.push_back(Loc);
}

// Replace the existing wedge with the pruned version.
if (ChangedThisWedge) {
FnVarLocs.setWedge(&I, std::move(NewDefs));
NumWedgesChanged++;
Changed = true;
}
// Replace the existing wedge with the pruned version.
if (ChangedThisWedge) {
FnVarLocs.setWedge(WedgePosition, std::move(NewDefs));
NumWedgesChanged++;
Changed = true;
}
};
for (DPValue &DPV : I.getDbgValueRange())
HandleLocsForWedge(&DPV);
HandleLocsForWedge(&I);
}

return Changed;
Expand Down Expand Up @@ -2726,6 +2749,9 @@ static DenseSet<DebugAggregate> findVarsWithStackSlot(Function &Fn) {
for (DbgAssignIntrinsic *DAI : at::getAssignmentMarkers(&I)) {
Result.insert({DAI->getVariable(), DAI->getDebugLoc().getInlinedAt()});
}
for (DPValue *DPV : at::getDPVAssignmentMarkers(&I)) {
Result.insert({DPV->getVariable(), DPV->getDebugLoc().getInlinedAt()});
}
}
}
return Result;
Expand Down
5 changes: 5 additions & 0 deletions llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1241,6 +1241,11 @@ void SelectionDAGBuilder::visitDbgInfo(const Instruction &I) {
It->Expr, Vals.size() > 1, It->DL, SDNodeOrder);
}
}
// We must early-exit here to prevent any DPValues from being emitted below,
// as we have just emitted the debug values resulting from assignment
// tracking analysis, making any existing DPValues redundant (and probably
// less correct).
return;
Copy link
Contributor

Choose a reason for hiding this comment

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

Please can you add a comment, either here or the top of the outer if that explains we don't need to look at DPValues when assignment tracking analysis has given us location info.

}

// Is there is any debug-info attached to this instruction, in the form of
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
; RUN: llc %s -stop-after=finalize-isel -o - | FileCheck %s


; RUN: llc --try-experimental-debuginfo-iterators %s -stop-after=finalize-isel -o - | FileCheck %s

;; Hand written. Check AssignmentTrackingAnalysis doesn't try to get the size
;; of scalable vectors (which causes an assertion failure).

Expand Down
4 changes: 4 additions & 0 deletions llvm/test/DebugInfo/assignment-tracking/X86/DSE.ll
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
; RUN: llc %s -stop-before=finalize-isel -o - \
; RUN: | FileCheck %s


; RUN: llc --try-experimental-debuginfo-iterators %s -stop-before=finalize-isel -o - \
; RUN: | FileCheck %s

; Check basic lowering behaviour of dbg.assign intrinsics. The first
; assignment to `local`, which has been DSE'd, should be represented with a
; constant value DBG_VALUE. The second assignment should have a DBG_VALUE
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
; RUN: llc %s -stop-after=finalize-isel -o - \
; RUN: | FileCheck %s --implicit-check-not=DBG_


; RUN: llc --try-experimental-debuginfo-iterators %s -stop-after=finalize-isel -o - \
; RUN: | FileCheck %s --implicit-check-not=DBG_

;; Check that SelectionDAG downgrades dbg.assigns to dbg.values if assignment
;; tracking isn't enabled (e.g. if the module flag
;; "debug-info-assignment-tracking" is missing / false).
Expand Down
4 changes: 4 additions & 0 deletions llvm/test/DebugInfo/assignment-tracking/X86/coalesce-cfg.ll
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
; RUN: llc %s -o - -stop-after=finalize-isel \
; RUN: | FileCheck %s --implicit-check-not=DBG_


; RUN: llc --try-experimental-debuginfo-iterators %s -o - -stop-after=finalize-isel \
; RUN: | FileCheck %s --implicit-check-not=DBG_

;; Test coalescing of contiguous fragments in adjacent location definitions.
;; Further details and check directives inline.

Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
; RUN: llc %s -stop-after=finalize-isel -o - \
; RUN: | FileCheck %s


; RUN: llc --try-experimental-debuginfo-iterators %s -stop-after=finalize-isel -o - \
; RUN: | FileCheck %s

;; Hand written test because the scenario is unlikely. Check that the "value"
;; of a debug def PHIs is "undef" (because we don't actually track PHIs).
;;
Expand Down
Loading