[RemoveDIs][DebugInfo] Handle DPVAssign in most transforms #78986
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-transforms @llvm/pr-subscribers-llvm-ir Author: Stephen Tozer (SLTozer) ChangesThis patch trivially updates various opt passes to handle DPVAssigns. In all cases, this means some combination of generifying existing code to handle DPValues and DbgAssignIntrinsics, iterating over DPValues where previously we did not, or duplicating code for DbgAssignIntrinsics to the equivalent DPValue function (in inlining and salvageDebugInfo). Patch is 29.30 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/78986.diff 12 Files Affected:
diff --git a/llvm/include/llvm/IR/DebugInfo.h b/llvm/include/llvm/IR/DebugInfo.h
index 55b8ce0998413b..aade822002e7f9 100644
--- a/llvm/include/llvm/IR/DebugInfo.h
+++ b/llvm/include/llvm/IR/DebugInfo.h
@@ -193,6 +193,12 @@ inline AssignmentInstRange getAssignmentInsts(const DbgAssignIntrinsic *DAI) {
return getAssignmentInsts(DAI->getAssignID());
}
+inline SmallVector<DPValue *> getDPVAssignmentMarkers(const Instruction *Inst) {
+ if (auto *ID = Inst->getMetadata(LLVMContext::MD_DIAssignID))
+ return cast<DIAssignID>(ID)->getAllDPValueUsers();
+ return {};
+}
+
//
// Utilities for enumerating llvm.dbg.assign intrinsic from an assignment ID.
//
diff --git a/llvm/lib/Transforms/Coroutines/CoroFrame.cpp b/llvm/lib/Transforms/Coroutines/CoroFrame.cpp
index db09337215c10f..e69c718f0ae3ac 100644
--- a/llvm/lib/Transforms/Coroutines/CoroFrame.cpp
+++ b/llvm/lib/Transforms/Coroutines/CoroFrame.cpp
@@ -963,7 +963,7 @@ static void cacheDIVar(FrameDataInfo &FrameData,
if (DIVarCache.contains(V))
continue;
- auto CacheIt = [&DIVarCache, V](auto Container) {
+ auto CacheIt = [&DIVarCache, V](const auto &Container) {
auto *I = llvm::find_if(Container, [](auto *DDI) {
return DDI->getExpression()->getNumElements() == 0;
});
@@ -1868,7 +1868,7 @@ static void insertSpills(const FrameDataInfo &FrameData, coro::Shape &Shape) {
// alias.
if (F->getSubprogram()) {
auto *CurDef = Def;
- while (DIs.empty() && isa<LoadInst>(CurDef)) {
+ while (DIs.empty() && DPVs.empty() && isa<LoadInst>(CurDef)) {
auto *LdInst = cast<LoadInst>(CurDef);
// Only consider ptr to ptr same type load.
if (LdInst->getPointerOperandType() != LdInst->getType())
@@ -2966,7 +2966,7 @@ void coro::salvageDebugInfo(
Function *F = DPV.getFunction();
// Follow the pointer arithmetic all the way to the incoming
// function argument and convert into a DIExpression.
- bool SkipOutermostLoad = DPV.getType() == DPValue::LocationType::Declare;
+ bool SkipOutermostLoad = DPV.isDbgDeclare();
Value *OriginalStorage = DPV.getVariableLocationOp(0);
auto SalvagedInfo = ::salvageDebugInfoImpl(
diff --git a/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp b/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp
index 64fbd5543a9e20..a647be2d26c761 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp
@@ -282,10 +282,12 @@ Instruction *InstCombinerImpl::SimplifyAnyMemSet(AnyMemSetInst *MI) {
Constant *FillVal = ConstantInt::get(ITy, Fill);
StoreInst *S = Builder.CreateStore(FillVal, Dest, MI->isVolatile());
S->copyMetadata(*MI, LLVMContext::MD_DIAssignID);
- for (auto *DAI : at::getAssignmentMarkers(S)) {
- if (llvm::is_contained(DAI->location_ops(), FillC))
- DAI->replaceVariableLocationOp(FillC, FillVal);
- }
+ auto replaceOpForAssignmentMarkers = [FillC, FillVal](auto *DbgAssign) {
+ if (llvm::is_contained(DbgAssign->location_ops(), FillC))
+ DbgAssign->replaceVariableLocationOp(FillC, FillVal);
+ };
+ for_each(at::getAssignmentMarkers(S), replaceOpForAssignmentMarkers);
+ for_each(at::getDPVAssignmentMarkers(S), replaceOpForAssignmentMarkers);
S->setAlignment(Alignment);
if (isa<AtomicMemSetInst>(MI))
diff --git a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
index 45f06e3045599b..249f4a7710e046 100644
--- a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
@@ -4522,7 +4522,7 @@ bool InstCombinerImpl::run() {
if (isa<PHINode>(I)) // PHI -> Non-PHI
InsertPos = InstParent->getFirstInsertionPt();
else // Non-PHI -> PHI
- InsertPos = InstParent->getFirstNonPHI()->getIterator();
+ InsertPos = InstParent->getFirstNonPHIIt();
}
Result->insertInto(InstParent, InsertPos);
diff --git a/llvm/lib/Transforms/Scalar/ADCE.cpp b/llvm/lib/Transforms/Scalar/ADCE.cpp
index 9af275a9f4e204..4291f6e425f4cf 100644
--- a/llvm/lib/Transforms/Scalar/ADCE.cpp
+++ b/llvm/lib/Transforms/Scalar/ADCE.cpp
@@ -549,6 +549,9 @@ ADCEChanged AggressiveDeadCodeElimination::removeDeadInstructions() {
// like the rest of this loop does. Extending support to assignment tracking
// is future work.
for (DPValue &DPV : make_early_inc_range(I.getDbgValueRange())) {
+ if (DPV.isDbgAssign())
+ if (!at::getAssignmentInsts(&DPV).empty())
+ continue;
if (AliveScopes.count(DPV.getDebugLoc()->getScope()))
continue;
I.dropOneDbgValue(&DPV);
diff --git a/llvm/lib/Transforms/Scalar/DeadStoreElimination.cpp b/llvm/lib/Transforms/Scalar/DeadStoreElimination.cpp
index 008dcc53fd44fc..11a91bfbe5baff 100644
--- a/llvm/lib/Transforms/Scalar/DeadStoreElimination.cpp
+++ b/llvm/lib/Transforms/Scalar/DeadStoreElimination.cpp
@@ -488,27 +488,27 @@ static void shortenAssignment(Instruction *Inst, Value *OriginalDest,
uint64_t DeadSliceSizeInBits = OldSizeInBits - NewSizeInBits;
uint64_t DeadSliceOffsetInBits =
OldOffsetInBits + (IsOverwriteEnd ? NewSizeInBits : 0);
- auto SetDeadFragExpr = [](DbgAssignIntrinsic *DAI,
+ auto SetDeadFragExpr = [](auto *Assign,
DIExpression::FragmentInfo DeadFragment) {
// createFragmentExpression expects an offset relative to the existing
// fragment offset if there is one.
uint64_t RelativeOffset = DeadFragment.OffsetInBits -
- DAI->getExpression()
+ Assign->getExpression()
->getFragmentInfo()
.value_or(DIExpression::FragmentInfo(0, 0))
.OffsetInBits;
if (auto NewExpr = DIExpression::createFragmentExpression(
- DAI->getExpression(), RelativeOffset, DeadFragment.SizeInBits)) {
- DAI->setExpression(*NewExpr);
+ Assign->getExpression(), RelativeOffset, DeadFragment.SizeInBits)) {
+ Assign->setExpression(*NewExpr);
return;
}
// Failed to create a fragment expression for this so discard the value,
// making this a kill location.
auto *Expr = *DIExpression::createFragmentExpression(
- DIExpression::get(DAI->getContext(), std::nullopt),
+ DIExpression::get(Assign->getContext(), std::nullopt),
DeadFragment.OffsetInBits, DeadFragment.SizeInBits);
- DAI->setExpression(Expr);
- DAI->setKillLocation();
+ Assign->setExpression(Expr);
+ Assign->setKillLocation();
};
// A DIAssignID to use so that the inserted dbg.assign intrinsics do not
@@ -526,32 +526,35 @@ static void shortenAssignment(Instruction *Inst, Value *OriginalDest,
// returned by getAssignmentMarkers so save a copy of the markers to iterate
// over.
auto LinkedRange = at::getAssignmentMarkers(Inst);
+ SmallVector<DPValue *> LinkedDPVAssigns = at::getDPVAssignmentMarkers(Inst);
SmallVector<DbgAssignIntrinsic *> Linked(LinkedRange.begin(),
LinkedRange.end());
- for (auto *DAI : Linked) {
+ auto InsertAssignForOverlap = [&](auto *Assign) {
std::optional<DIExpression::FragmentInfo> NewFragment;
if (!at::calculateFragmentIntersect(DL, OriginalDest, DeadSliceOffsetInBits,
- DeadSliceSizeInBits, DAI,
+ DeadSliceSizeInBits, Assign,
NewFragment) ||
!NewFragment) {
// We couldn't calculate the intersecting fragment for some reason. Be
// cautious and unlink the whole assignment from the store.
- DAI->setKillAddress();
- DAI->setAssignId(GetDeadLink());
- continue;
+ Assign->setKillAddress();
+ Assign->setAssignId(GetDeadLink());
+ return;
}
// No intersect.
if (NewFragment->SizeInBits == 0)
- continue;
+ return;
// Fragments overlap: insert a new dbg.assign for this dead part.
- auto *NewAssign = cast<DbgAssignIntrinsic>(DAI->clone());
- NewAssign->insertAfter(DAI);
+ auto *NewAssign = static_cast<decltype(Assign)>(Assign->clone());
+ NewAssign->insertAfter(Assign);
NewAssign->setAssignId(GetDeadLink());
if (NewFragment)
SetDeadFragExpr(NewAssign, *NewFragment);
NewAssign->setKillAddress();
- }
+ };
+ for_each(Linked, InsertAssignForOverlap);
+ for_each(LinkedDPVAssigns, InsertAssignForOverlap);
}
static bool tryToShorten(Instruction *DeadI, int64_t &DeadStart,
diff --git a/llvm/lib/Transforms/Utils/InlineFunction.cpp b/llvm/lib/Transforms/Utils/InlineFunction.cpp
index 39d5f6e53c1de4..d4d4bf5ebdf36e 100644
--- a/llvm/lib/Transforms/Utils/InlineFunction.cpp
+++ b/llvm/lib/Transforms/Utils/InlineFunction.cpp
@@ -1789,13 +1789,15 @@ static at::StorageToVarsMap collectEscapedLocals(const DataLayout &DL,
continue;
// Find all local variables associated with the backing storage.
- for (auto *DAI : at::getAssignmentMarkers(Base)) {
+ auto CollectAssignsForStorage = [&](auto *DbgAssign) {
// Skip variables from inlined functions - they are not local variables.
- if (DAI->getDebugLoc().getInlinedAt())
- continue;
- LLVM_DEBUG(errs() << " > DEF : " << *DAI << "\n");
- EscapedLocals[Base].insert(at::VarRecord(DAI));
- }
+ if (DbgAssign->getDebugLoc().getInlinedAt())
+ return;
+ LLVM_DEBUG(errs() << " > DEF : " << *DbgAssign << "\n");
+ EscapedLocals[Base].insert(at::VarRecord(DbgAssign));
+ };
+ for_each(at::getAssignmentMarkers(Base), CollectAssignsForStorage);
+ for_each(at::getDPVAssignmentMarkers(Base), CollectAssignsForStorage);
}
return EscapedLocals;
}
@@ -1827,6 +1829,10 @@ static void fixupAssignments(Function::iterator Start, Function::iterator End) {
// attachment or use, replace it with a new version.
for (auto BBI = Start; BBI != End; ++BBI) {
for (Instruction &I : *BBI) {
+ for (DPValue &DPV : I.getDbgValueRange()) {
+ if (DPV.isDbgAssign())
+ DPV.setAssignId(GetNewID(DPV.getAssignID()));
+ }
if (auto *ID = I.getMetadata(LLVMContext::MD_DIAssignID))
I.setMetadata(LLVMContext::MD_DIAssignID, GetNewID(ID));
else if (auto *DAI = dyn_cast<DbgAssignIntrinsic>(&I))
diff --git a/llvm/lib/Transforms/Utils/Local.cpp b/llvm/lib/Transforms/Utils/Local.cpp
index 2a1ac85ee55bf5..459e3d98059283 100644
--- a/llvm/lib/Transforms/Utils/Local.cpp
+++ b/llvm/lib/Transforms/Utils/Local.cpp
@@ -1753,7 +1753,7 @@ void llvm::ConvertDebugDeclareToDebugValue(DbgVariableIntrinsic *DII,
void llvm::ConvertDebugDeclareToDebugValue(DPValue *DPV, StoreInst *SI,
DIBuilder &Builder) {
- assert(DPV->isAddressOfVariable());
+ assert(DPV->isAddressOfVariable() || DPV->isDbgAssign());
auto *DIVar = DPV->getVariable();
assert(DIVar && "Missing variable");
auto *DIExpr = DPV->getExpression();
@@ -2189,14 +2189,13 @@ void llvm::salvageDebugInfo(Instruction &I) {
salvageDebugInfoForDbgValues(I, DbgUsers, DPUsers);
}
-/// Salvage the address component of \p DAI.
-static void salvageDbgAssignAddress(DbgAssignIntrinsic *DAI) {
- Instruction *I = dyn_cast<Instruction>(DAI->getAddress());
+template <typename T> static void salvageDbgAssignAddress(T *Assign) {
+ Instruction *I = dyn_cast<Instruction>(Assign->getAddress());
// Only instructions can be salvaged at the moment.
if (!I)
return;
- assert(!DAI->getAddressExpression()->getFragmentInfo().has_value() &&
+ assert(!Assign->getAddressExpression()->getFragmentInfo().has_value() &&
"address-expression shouldn't have fragment info");
// The address component of a dbg.assign cannot be variadic.
@@ -2210,16 +2209,16 @@ static void salvageDbgAssignAddress(DbgAssignIntrinsic *DAI) {
return;
DIExpression *SalvagedExpr = DIExpression::appendOpsToArg(
- DAI->getAddressExpression(), Ops, 0, /*StackValue=*/false);
+ Assign->getAddressExpression(), Ops, 0, /*StackValue=*/false);
assert(!SalvagedExpr->getFragmentInfo().has_value() &&
"address-expression shouldn't have fragment info");
// Salvage succeeds if no additional values are required.
if (AdditionalValues.empty()) {
- DAI->setAddress(NewV);
- DAI->setAddressExpression(SalvagedExpr);
+ Assign->setAddress(NewV);
+ Assign->setAddressExpression(SalvagedExpr);
} else {
- DAI->setKillAddress();
+ Assign->setKillAddress();
}
}
@@ -2293,10 +2292,19 @@ void llvm::salvageDebugInfoForDbgValues(
}
// Duplicate of above block for DPValues.
for (auto *DPV : DPUsers) {
+ if (DPV->isDbgAssign()) {
+ if (DPV->getAddress() == &I) {
+ salvageDbgAssignAddress(DPV);
+ Salvaged = true;
+ }
+ if (DPV->getValue() != &I)
+ continue;
+ }
+
// Do not add DW_OP_stack_value for DbgDeclare and DbgAddr, because they
// are implicitly pointing out the value as a DWARF memory location
// description.
- bool StackValue = DPV->getType() == DPValue::LocationType::Value;
+ bool StackValue = DPV->getType() != DPValue::LocationType::Declare;
auto DPVLocation = DPV->location_ops();
assert(
is_contained(DPVLocation, &I) &&
@@ -2330,7 +2338,7 @@ void llvm::salvageDebugInfoForDbgValues(
SalvagedExpr->getNumElements() <= MaxExpressionSize;
if (AdditionalValues.empty() && IsValidSalvageExpr) {
DPV->setExpression(SalvagedExpr);
- } else if (DPV->getType() == DPValue::LocationType::Value &&
+ } else if (DPV->getType() != DPValue::LocationType::Declare &&
IsValidSalvageExpr &&
DPV->getNumVariableLocationOps() + AdditionalValues.size() <=
MaxDebugArgs) {
@@ -2340,8 +2348,7 @@ void llvm::salvageDebugInfoForDbgValues(
// currently only valid for stack value expressions.
// Also do not salvage if the resulting DIArgList would contain an
// unreasonably large number of values.
- Value *Undef = UndefValue::get(I.getOperand(0)->getType());
- DPV->replaceVariableLocationOp(I.getOperand(0), Undef);
+ DPV->setKillLocation();
}
LLVM_DEBUG(dbgs() << "SALVAGE: " << DPV << '\n');
Salvaged = true;
diff --git a/llvm/lib/Transforms/Utils/PromoteMemoryToRegister.cpp b/llvm/lib/Transforms/Utils/PromoteMemoryToRegister.cpp
index 717b6d301c8c90..9744a32a4f7c8a 100644
--- a/llvm/lib/Transforms/Utils/PromoteMemoryToRegister.cpp
+++ b/llvm/lib/Transforms/Utils/PromoteMemoryToRegister.cpp
@@ -101,12 +101,29 @@ bool llvm::isAllocaPromotable(const AllocaInst *AI) {
namespace {
+static DPValue *createDebugValue(DIBuilder &DIB, Value *NewValue,
+ DILocalVariable *Variable,
+ DIExpression *Expression, const DILocation *DI,
+ DPValue *InsertBefore) {
+ (void)DIB;
+ DPValue::createDPValue(NewValue, Variable, Expression, DI, *InsertBefore);
+}
+static DbgValueInst *createDebugValue(DIBuilder &DIB, Value *NewValue,
+ DILocalVariable *Variable,
+ DIExpression *Expression,
+ const DILocation *DI,
+ Instruction *InsertBefore) {
+ return static_cast<DbgValueInst *>(DIB.insertDbgValueIntrinsic(
+ NewValue, Variable, Expression, DI, InsertBefore));
+}
+
/// Helper for updating assignment tracking debug info when promoting allocas.
class AssignmentTrackingInfo {
/// DbgAssignIntrinsics linked to the alloca with at most one per variable
/// fragment. (i.e. not be a comprehensive set if there are multiple
/// dbg.assigns for one variable fragment).
SmallVector<DbgVariableIntrinsic *> DbgAssigns;
+ SmallVector<DPValue *> DPVAssigns;
public:
void init(AllocaInst *AI) {
@@ -115,16 +132,21 @@ class AssignmentTrackingInfo {
if (Vars.insert(DebugVariable(DAI)).second)
DbgAssigns.push_back(DAI);
}
+ for (DPValue *DPV : at::getDPVAssignmentMarkers(AI)) {
+ if (Vars.insert(DebugVariable(DPV)).second)
+ DPVAssigns.push_back(DPV);
+ }
}
/// Update assignment tracking debug info given for the to-be-deleted store
/// \p ToDelete that stores to this alloca.
- void updateForDeletedStore(
- StoreInst *ToDelete, DIBuilder &DIB,
- SmallSet<DbgAssignIntrinsic *, 8> *DbgAssignsToDelete) const {
+ void
+ updateForDeletedStore(StoreInst *ToDelete, DIBuilder &DIB,
+ SmallSet<DbgAssignIntrinsic *, 8> *DbgAssignsToDelete,
+ SmallSet<DPValue *, 8> *DPVAssignsToDelete) const {
// There's nothing to do if the alloca doesn't have any variables using
// assignment tracking.
- if (DbgAssigns.empty())
+ if (DbgAssigns.empty() && DPVAssigns.empty())
return;
// Insert a dbg.value where the linked dbg.assign is and remember to delete
@@ -134,13 +156,17 @@ class AssignmentTrackingInfo {
// dbg.assign for each variable fragment for the untracked store handling
// (after this loop).
SmallSet<DebugVariableAggregate, 2> VarHasDbgAssignForStore;
- for (DbgAssignIntrinsic *DAI : at::getAssignmentMarkers(ToDelete)) {
- VarHasDbgAssignForStore.insert(DebugVariableAggregate(DAI));
- DbgAssignsToDelete->insert(DAI);
- DIB.insertDbgValueIntrinsic(DAI->getValue(), DAI->getVariable(),
- DAI->getExpression(), DAI->getDebugLoc(),
- DAI);
- }
+ auto InsertValueForAssign = [&](auto *DbgAssign, auto *&AssignList) {
+ VarHasDbgAssignForStore.insert(DebugVariableAggregate(DbgAssign));
+ AssignList->insert(DbgAssign);
+ createDebugValue(DIB, DbgAssign->getValue(), DbgAssign->getVariable(),
+ DbgAssign->getExpression(), DbgAssign->getDebugLoc(),
+ DbgAssign);
+ };
+ for (auto *Assign : at::getAssignmentMarkers(ToDelete))
+ InsertValueForAssign(Assign, DbgAssignsToDelete);
+ for (auto *Assign : at::getDPVAssignmentMarkers(ToDelete))
+ InsertValueForAssign(Assign, DPVAssignsToDelete);
// It's possible for variables using assignment tracking to have no
// dbg.assign linked to this store. These are variables in DbgAssigns that
@@ -150,11 +176,13 @@ class AssignmentTrackingInfo {
// cannot be represented using assignment tracking (non-const offset or
// size) or one that is trackable but has had its DIAssignID attachment
// dropped accidentally.
- for (auto *DAI : DbgAssigns) {
- if (VarHasDbgAssignForStore.contains(DebugVariableAggregate(DAI)))
- continue;
- ConvertDebugDeclareToDebugValue(DAI, ToDelete, DIB);
- }
+ auto ConvertUnlinkedAssignToValue = [&](auto *Assign) {
+ if (VarHasDbgAssignForStore.contains(DebugVariableAggregate(Assign)))
+ return;
+ ConvertDebugDeclareToDebugValue(Assign, ToDelete, DIB);
+ };
+ for_each(DbgAssigns, ConvertUnlinkedAssignToValue);
+ for_each(DPVAssigns, ConvertUnlinkedAssignToValue);
}
/// Update assignment tracking debug info given for the newly inserted PHI \p
@@ -165,10 +193,15 @@ class AssignmentTrackingInfo {
// debug-phi.
for (auto *DAI : DbgAssigns)
ConvertDebugDeclareToDebugValue(DAI, NewPhi, DIB);
+ for (auto *DPV : DPVAssigns)
+ ConvertDebugDeclareToDebugValue(DPV, NewPhi, DIB);
}
- void clear() { DbgAssigns.clear(); }
- bool empty() { return DbgAssigns.empty(); }
+ void clear() {
+ DbgAssigns.clear();
+ DPVAssigns.clear();
+ }
+ bool empty() { return DbgAssigns.empty() && DPVAssigns.empty(); }
};
struct AllocaInfo {
@@ -229,11 +262,15 @@ struct AllocaInfo {
}
}
DbgUserVec AllDbgUsers;
- findDbgUsers(AllDbgUsers, AI, &DPUsers);
+ SmallVector<DPValue *> AllDPUsers;
+ findDbgUsers(AllDbgUsers, AI, &AllDPUsers);
std::copy_if(AllDbgUsers.begin(), AllDbgUsers.end(),
std::back_inserter(DbgUsers), [](DbgVariableIntrinsic *DII) {
return !isa<DbgAssignIntrinsic>(DII);
});
+...
[truncated]
|
OCHyams
reviewed
Jan 22, 2024
| @@ -1868,7 +1868,7 @@ static void insertSpills(const FrameDataInfo &FrameData, coro::Shape &Shape) { | |||
| // alias. | |||
| if (F->getSubprogram()) { | |||
| auto *CurDef = Def; | |||
| while (DIs.empty() && isa<LoadInst>(CurDef)) { | |||
| while (DIs.empty() && DPVs.empty() && isa<LoadInst>(CurDef)) { | |||
| @@ -549,6 +549,9 @@ ADCEChanged AggressiveDeadCodeElimination::removeDeadInstructions() { | |||
| // like the rest of this loop does. Extending support to assignment tracking | |||
| // is future work. | |||
| for (DPValue &DPV : make_early_inc_range(I.getDbgValueRange())) { | |||
| if (DPV.isDbgAssign()) | |||
There was a problem hiding this comment.
Please can you copy the comment from the intrinsic loop below.
| DbgAssign->getExpression(), DbgAssign->getDebugLoc(), | ||
| DbgAssign); | ||
| }; | ||
| for (auto *Assign : at::getAssignmentMarkers(ToDelete)) |
There was a problem hiding this comment.
I decided against it in this case, because we don't have a function that takes a single argument - the only way to use for_each in this case would be a separate lambda for intrinsics and DPValues, which would make the code less simple rather than more.
Comment on lines
+1764
to
+1768
| // Also clone DPValues from the existing terminator, and all others (to | ||
| // duplicate existing hoisting behaviour). | ||
| NT->cloneDebugInfoFrom(I1); | ||
| for (Instruction *OtherSuccTI : OtherSuccTIs) | ||
| NT->cloneDebugInfoFrom(OtherSuccTI); |
There was a problem hiding this comment.
Which test (assuming tests will be added shortly) covers this change?
There was a problem hiding this comment.
This should probably be split out of here - I think this is a general DPValue bug that I found while validating the assignment tracking patches; I'll create a separate patch with a reduced reproducer as the test case.
There was a problem hiding this comment.
Nevermind, I was wrong - DebugInfo/Generic/assignment-tracking/simplifycfg/empty-block.ll fails without this! Will re-add this to the patch (and also make sure that test is included).
SLTozer
force-pushed
the
gbtozers/ddd-at-9-trivial-transforms
branch
from
6a8d3de
to
32ff694
Compare
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
SLTozer
force-pushed
the
gbtozers/ddd-at-9-trivial-transforms
branch
from
0e876bf
to
aeca37d
Compare
SLTozer
force-pushed
the
gbtozers/ddd-at-9-trivial-transforms
branch
from
aeca37d
to
06a4856
Compare
OCHyams
approved these changes
Jan 23, 2024
| if (DPV.isDbgAssign() && IsInvalidLocation(DPV.getAddress())) { | ||
| DPVsToDelete.push_back(&DPV); | ||
| continue; | ||
| } |
There was a problem hiding this comment.
there's a comment a few lines up that needs deleting // FIXME: support dbg.assign form of DPValues.
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.
This patch trivially updates various opt passes to handle DPVAssigns. In all cases, this means some combination of generifying existing code to handle DPValues and DbgAssignIntrinsics, iterating over DPValues where previously we did not, or duplicating code for DbgAssignIntrinsics to the equivalent DPValue function (in inlining and salvageDebugInfo).