[RemoveDIs][DebugInfo] Update SROA to handle DPVAssigns #78475
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@llvm/pr-subscribers-llvm-ir @llvm/pr-subscribers-debuginfo Author: Stephen Tozer (SLTozer) ChangesSROA needs to update llvm.dbg.assign intrinsics when it migrates debug info in response to alloca splitting; this patch updates the debug info migration code to handle DPVAssigns as well, making use of generic code to avoid duplication as much as possible. Full diff: https://github.com/llvm/llvm-project/pull/78475.diff 2 Files Affected:
diff --git a/llvm/include/llvm/IR/DebugInfo.h b/llvm/include/llvm/IR/DebugInfo.h
index e634f4bd2f5aaec..626f5d4b3191352 100644
--- a/llvm/include/llvm/IR/DebugInfo.h
+++ b/llvm/include/llvm/IR/DebugInfo.h
@@ -224,6 +224,12 @@ inline AssignmentMarkerRange getAssignmentMarkers(const Instruction *Inst) {
return make_range(Value::user_iterator(), Value::user_iterator());
}
+inline SmallVector<DPValue *> getDPVAssignmentMarkers(const Instruction *Inst) {
+ if (auto *ID = Inst->getMetadata(LLVMContext::MD_DIAssignID))
+ return cast<DIAssignID>(ID)->getAllDPValueUsers();
+ return {};
+}
+
/// Delete the llvm.dbg.assign intrinsics linked to \p Inst.
void deleteAssignmentMarkers(const Instruction *Inst);
diff --git a/llvm/lib/Transforms/Scalar/SROA.cpp b/llvm/lib/Transforms/Scalar/SROA.cpp
index adf87ebb69f546b..79a75afbb6878b9 100644
--- a/llvm/lib/Transforms/Scalar/SROA.cpp
+++ b/llvm/lib/Transforms/Scalar/SROA.cpp
@@ -319,6 +319,29 @@ static DebugVariable getAggregateVariable(DbgVariableIntrinsic *DVI) {
return DebugVariable(DVI->getVariable(), std::nullopt,
DVI->getDebugLoc().getInlinedAt());
}
+static DebugVariable getAggregateVariable(DPValue *DPV) {
+ return DebugVariable(DPV->getVariable(), std::nullopt,
+ DPV->getDebugLoc().getInlinedAt());
+}
+
+static DPValue *createLinkedAssign(DPValue *, DIBuilder &DIB,
+ Instruction *LinkedInstr, Value *NewValue,
+ DILocalVariable *Variable,
+ DIExpression *Expression, Value *Address,
+ DIExpression *AddressExpression,
+ const DILocation *DI) {
+ (void)DIB;
+ return DPValue::createLinkedDPVAssign(LinkedInstr, NewValue, Variable,
+ Expression, Address, AddressExpression,
+ DI);
+}
+static DbgAssignIntrinsic *createLinkedAssign(
+ DbgAssignIntrinsic *, DIBuilder &DIB, Instruction *LinkedInstr,
+ Value *NewValue, DILocalVariable *Variable, DIExpression *Expression,
+ Value *Address, DIExpression *AddressExpression, const DILocation *DI) {
+ return DIB.insertDbgAssign(LinkedInstr, NewValue, Variable, Expression,
+ Address, AddressExpression, DI);
+}
/// Find linked dbg.assign and generate a new one with the correct
/// FragmentInfo. Link Inst to the new dbg.assign. If Value is nullptr the
@@ -340,8 +363,9 @@ static void migrateDebugInfo(AllocaInst *OldAlloca, bool IsSplit,
Instruction *Inst, Value *Dest, Value *Value,
const DataLayout &DL) {
auto MarkerRange = at::getAssignmentMarkers(OldInst);
+ auto DPVAssignMarkerRange = at::getDPVAssignmentMarkers(OldInst);
// Nothing to do if OldInst has no linked dbg.assign intrinsics.
- if (MarkerRange.empty())
+ if (MarkerRange.empty() && DPVAssignMarkerRange.empty())
return;
LLVM_DEBUG(dbgs() << " migrateDebugInfo\n");
@@ -362,6 +386,9 @@ static void migrateDebugInfo(AllocaInst *OldAlloca, bool IsSplit,
for (auto *DAI : at::getAssignmentMarkers(OldAlloca))
BaseFragments[getAggregateVariable(DAI)] =
DAI->getExpression()->getFragmentInfo();
+ for (auto *DPV : at::getDPVAssignmentMarkers(OldAlloca))
+ BaseFragments[getAggregateVariable(DPV)] =
+ DPV->getExpression()->getFragmentInfo();
// The new inst needs a DIAssignID unique metadata tag (if OldInst has
// one). It shouldn't already have one: assert this assumption.
@@ -371,7 +398,7 @@ static void migrateDebugInfo(AllocaInst *OldAlloca, bool IsSplit,
DIBuilder DIB(*OldInst->getModule(), /*AllowUnresolved*/ false);
assert(OldAlloca->isStaticAlloca());
- for (DbgAssignIntrinsic *DbgAssign : MarkerRange) {
+ auto MigrateDbgAssign = [&](auto DbgAssign) {
LLVM_DEBUG(dbgs() << " existing dbg.assign is: " << *DbgAssign
<< "\n");
auto *Expr = DbgAssign->getExpression();
@@ -382,7 +409,7 @@ static void migrateDebugInfo(AllocaInst *OldAlloca, bool IsSplit,
{
auto R = BaseFragments.find(getAggregateVariable(DbgAssign));
if (R == BaseFragments.end())
- continue;
+ return;
BaseFragment = R->second;
}
std::optional<DIExpression::FragmentInfo> CurrentFragment =
@@ -393,7 +420,7 @@ static void migrateDebugInfo(AllocaInst *OldAlloca, bool IsSplit,
BaseFragment, CurrentFragment, NewFragment);
if (Result == Skip)
- continue;
+ return;
if (Result == UseFrag && !(NewFragment == CurrentFragment)) {
if (CurrentFragment) {
// Rewrite NewFragment to be relative to the existing one (this is
@@ -425,9 +452,10 @@ static void migrateDebugInfo(AllocaInst *OldAlloca, bool IsSplit,
}
::Value *NewValue = Value ? Value : DbgAssign->getValue();
- auto *NewAssign = DIB.insertDbgAssign(
- Inst, NewValue, DbgAssign->getVariable(), Expr, Dest,
- DIExpression::get(Ctx, std::nullopt), DbgAssign->getDebugLoc());
+ auto *NewAssign = createLinkedAssign(
+ DbgAssign, DIB, Inst, NewValue, DbgAssign->getVariable(), Expr, Dest,
+ DIExpression::get(Expr->getContext(), std::nullopt),
+ DbgAssign->getDebugLoc());
// If we've updated the value but the original dbg.assign has an arglist
// then kill it now - we can't use the requested new value.
@@ -461,9 +489,11 @@ static void migrateDebugInfo(AllocaInst *OldAlloca, bool IsSplit,
NewAssign->moveBefore(DbgAssign);
NewAssign->setDebugLoc(DbgAssign->getDebugLoc());
- LLVM_DEBUG(dbgs() << "Created new assign intrinsic: " << *NewAssign
- << "\n");
- }
+ LLVM_DEBUG(dbgs() << "Created new assign: " << *NewAssign << "\n");
+ };
+
+ for_each(MarkerRange, MigrateDbgAssign);
+ for_each(DPVAssignMarkerRange, MigrateDbgAssign);
}
namespace {
@@ -3108,6 +3138,7 @@ class AllocaSliceRewriter : public InstVisitor<AllocaSliceRewriter, bool> {
// emit dbg.assign intrinsics for mem intrinsics storing through non-
// constant geps, or storing a variable number of bytes.
assert(at::getAssignmentMarkers(&II).empty() &&
+ at::getDPVAssignmentMarkers(&II).empty() &&
"AT: Unexpected link to non-const GEP");
deleteIfTriviallyDead(OldPtr);
return false;
@@ -3254,11 +3285,13 @@ class AllocaSliceRewriter : public InstVisitor<AllocaSliceRewriter, bool> {
Value *AdjustedPtr = getNewAllocaSlicePtr(IRB, OldPtr->getType());
if (IsDest) {
// Update the address component of linked dbg.assigns.
- for (auto *DAI : at::getAssignmentMarkers(&II)) {
- if (llvm::is_contained(DAI->location_ops(), II.getDest()) ||
- DAI->getAddress() == II.getDest())
- DAI->replaceVariableLocationOp(II.getDest(), AdjustedPtr);
- }
+ auto UpdateAssignAddress = [&](auto *DbgAssign) {
+ if (llvm::is_contained(DbgAssign->location_ops(), II.getDest()) ||
+ DbgAssign->getAddress() == II.getDest())
+ DbgAssign->replaceVariableLocationOp(II.getDest(), AdjustedPtr);
+ };
+ for_each(at::getAssignmentMarkers(&II), UpdateAssignAddress);
+ for_each(at::getDPVAssignmentMarkers(&II), UpdateAssignAddress);
II.setDest(AdjustedPtr);
II.setDestAlignment(SliceAlign);
} else {
@@ -3842,6 +3875,7 @@ class AggLoadStoreRewriter : public InstVisitor<AggLoadStoreRewriter, bool> {
DL);
} else {
assert(at::getAssignmentMarkers(Store).empty() &&
+ at::getDPVAssignmentMarkers(Store).empty() &&
"AT: unexpected debug.assign linked to store through "
"unbounded GEP");
}
@@ -4861,10 +4895,21 @@ static void insertNewDbgInst(DIBuilder &DIB, DPValue *Orig, AllocaInst *NewAddr,
DIExpression *NewFragmentExpr,
Instruction *BeforeInst) {
(void)DIB;
- DPValue *New = new DPValue(ValueAsMetadata::get(NewAddr), Orig->getVariable(),
- NewFragmentExpr, Orig->getDebugLoc(),
- DPValue::LocationType::Declare);
- BeforeInst->getParent()->insertDPValueBefore(New, BeforeInst->getIterator());
+ if (Orig->isDbgDeclare()) {
+ DPValue *DPV = DPValue::createDPVDeclare(NewAddr, Orig->getVariable(), NewFragmentExpr,
+ Orig->getDebugLoc());
+ BeforeInst->getParent()->insertDPValueBefore(DPV, BeforeInst->getIterator());
+ return;
+ }
+ if (!NewAddr->hasMetadata(LLVMContext::MD_DIAssignID)) {
+ NewAddr->setMetadata(LLVMContext::MD_DIAssignID,
+ DIAssignID::getDistinct(NewAddr->getContext()));
+ }
+ auto *NewAssign = DPValue::createLinkedDPVAssign(
+ NewAddr, Orig->getValue(), Orig->getVariable(), NewFragmentExpr, NewAddr,
+ Orig->getAddressExpression(), Orig->getDebugLoc());
+ LLVM_DEBUG(dbgs() << "Created new DPVAssign: " << *NewAssign << "\n");
+ (void)NewAssign;
}
/// Walks the slices of an alloca and form partitions based on them,
@@ -5042,6 +5087,7 @@ bool SROA::splitAlloca(AllocaInst &AI, AllocaSlices &AS) {
for_each(findDbgDeclares(&AI), MigrateOne);
for_each(findDPVDeclares(&AI), MigrateOne);
for_each(at::getAssignmentMarkers(&AI), MigrateOne);
+ for_each(at::getDPVAssignmentMarkers(&AI), MigrateOne);
return Changed;
}
@@ -5262,8 +5308,9 @@ std::pair<bool /*Changed*/, bool /*CFGChanged*/> SROA::runSROA(Function &F) {
"Should not have modified the CFG when told to preserve it.");
if (Changed && isAssignmentTrackingEnabled(*F.getParent())) {
- for (auto &BB : F)
+ for (auto &BB : F) {
RemoveRedundantDbgInstrs(&BB);
+ }
}
return {Changed, CFGChanged};
|
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
OCHyams
approved these changes
Jan 19, 2024
There was a problem hiding this comment.
Code changes all LGTM, but this needs tests too. What's your intended strategy for landing code/test changes?
FWIW For DPDeclares I updated the tests for each pass in the same patch the code was update (e.g., you would update SROA tests here), with the tests "rotten green" (not actually testing the DPDeclares) until the capstone patch that enabled dbg.declare -> DPDeclare landed.
| DPV->getDebugLoc().getInlinedAt()); | ||
| } | ||
|
|
||
| static DPValue *createLinkedAssign(DPValue *, DIBuilder &DIB, |
OCHyams
approved these changes
Jan 22, 2024
|
@SLTozer Hi Stephen, it looks as though your change may have caused a build bot failure here: |
|
Apologies, I see the fix has already been landed - thanks for catching this! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
SROA needs to update llvm.dbg.assign intrinsics when it migrates debug info in response to alloca splitting; this patch updates the debug info migration code to handle DPVAssigns as well, making use of generic code to avoid duplication as much as possible.