From 2722cf87c2d14019e35e21611207265fa25cb775 Mon Sep 17 00:00:00 2001 From: Carlos Alberto Enciso <47597242+CarlosAlbertoEnciso@users.noreply.github.com> Date: Fri, 5 Apr 2024 07:27:37 +0100 Subject: [PATCH 1/5] [Transforms] Debug values are not remapped when cloning. When cloning instructions from one basic block to another, the debug values are not remapped, in the same was as the normal instructions. --- llvm/include/llvm/IR/IntrinsicInst.h | 3 +- llvm/lib/IR/IntrinsicInst.cpp | 5 ++- llvm/lib/Transforms/Scalar/JumpThreading.cpp | 29 +++++++++++++++ llvm/lib/Transforms/Utils/CloneFunction.cpp | 29 +++++++++++++++ .../CallSiteSplitting/callsite-split-debug.ll | 36 ++++++++++++++++++- 5 files changed, 99 insertions(+), 3 deletions(-) diff --git a/llvm/include/llvm/IR/IntrinsicInst.h b/llvm/include/llvm/IR/IntrinsicInst.h index 4f22720f1c558..e74bf15a21b9a 100644 --- a/llvm/include/llvm/IR/IntrinsicInst.h +++ b/llvm/include/llvm/IR/IntrinsicInst.h @@ -311,7 +311,8 @@ class DbgVariableIntrinsic : public DbgInfoIntrinsic { Value *getVariableLocationOp(unsigned OpIdx) const; - void replaceVariableLocationOp(Value *OldValue, Value *NewValue); + void replaceVariableLocationOp(Value *OldValue, Value *NewValue, + bool AllowEmpty = false); void replaceVariableLocationOp(unsigned OpIdx, Value *NewValue); /// Adding a new location operand will always result in this intrinsic using /// an ArgList, and must always be accompanied by a new expression that uses diff --git a/llvm/lib/IR/IntrinsicInst.cpp b/llvm/lib/IR/IntrinsicInst.cpp index 89403e1d7fcb4..cff716a843099 100644 --- a/llvm/lib/IR/IntrinsicInst.cpp +++ b/llvm/lib/IR/IntrinsicInst.cpp @@ -119,7 +119,8 @@ static ValueAsMetadata *getAsMetadata(Value *V) { } void DbgVariableIntrinsic::replaceVariableLocationOp(Value *OldValue, - Value *NewValue) { + Value *NewValue, + bool AllowEmpty) { // If OldValue is used as the address part of a dbg.assign intrinsic replace // it with NewValue and return true. auto ReplaceDbgAssignAddress = [this, OldValue, NewValue]() -> bool { @@ -136,6 +137,8 @@ void DbgVariableIntrinsic::replaceVariableLocationOp(Value *OldValue, auto Locations = location_ops(); auto OldIt = find(Locations, OldValue); if (OldIt == Locations.end()) { + if (AllowEmpty) + return; assert(DbgAssignAddrReplaced && "OldValue must be dbg.assign addr if unused in DIArgList"); return; diff --git a/llvm/lib/Transforms/Scalar/JumpThreading.cpp b/llvm/lib/Transforms/Scalar/JumpThreading.cpp index ffcb511e6a831..d7936969011e6 100644 --- a/llvm/lib/Transforms/Scalar/JumpThreading.cpp +++ b/llvm/lib/Transforms/Scalar/JumpThreading.cpp @@ -2694,6 +2694,35 @@ bool JumpThreadingPass::duplicateCondBranchOnPHIIntoPred( New->setOperand(i, I->second); } + // Remap debug variable operands. + auto RemapDebugVariable = [](auto &Mapping, auto *Inst) { + auto RemapDebugOperands = [&Mapping](auto *DV, auto Set) { + for (auto *Op : Set) + if (Instruction *InstOp = dyn_cast(Op)) { + auto I = Mapping.find(InstOp); + if (I != Mapping.end()) + DV->replaceVariableLocationOp(Op, I->second, /*AllowEmpty=*/true); + } + }; + auto RemapAssignAddress = [&Mapping](auto *DA) { + if (Instruction *Addr = dyn_cast(DA->getAddress())) { + auto I = Mapping.find(Addr); + if (I != Mapping.end()) + DA->setAddress(I->second); + } + }; + if (auto DVI = dyn_cast(Inst)) + RemapDebugOperands(DVI, DVI->location_ops()); + if (auto DAI = dyn_cast(Inst)) + RemapAssignAddress(DAI); + for (DbgVariableRecord &DVR : filterDbgVars(Inst->getDbgRecordRange())) { + RemapDebugOperands(&DVR, DVR.location_ops()); + if (DVR.isDbgAssign()) + RemapAssignAddress(&DVR); + } + }; + RemapDebugVariable(ValueMapping, New); + // If this instruction can be simplified after the operands are updated, // just use the simplified value instead. This frequently happens due to // phi translation. diff --git a/llvm/lib/Transforms/Utils/CloneFunction.cpp b/llvm/lib/Transforms/Utils/CloneFunction.cpp index 3eac726994ae1..de58e1b759f96 100644 --- a/llvm/lib/Transforms/Utils/CloneFunction.cpp +++ b/llvm/lib/Transforms/Utils/CloneFunction.cpp @@ -1131,6 +1131,35 @@ BasicBlock *llvm::DuplicateInstructionsInSplitBetween( if (I != ValueMapping.end()) New->setOperand(i, I->second); } + + // Remap debug variable operands. + auto RemapDebugVariable = [](auto &Mapping, auto *Inst) { + auto RemapDebugOperands = [&Mapping](auto *DV, auto Set) { + for (auto *Op : Set) + if (Instruction *InstOp = dyn_cast(Op)) { + auto I = Mapping.find(InstOp); + if (I != Mapping.end()) + DV->replaceVariableLocationOp(Op, I->second, /*AllowEmpty=*/true); + } + }; + auto RemapAssignAddress = [&Mapping](auto *DA) { + if (Instruction *Addr = dyn_cast(DA->getAddress())) { + auto I = Mapping.find(Addr); + if (I != Mapping.end()) + DA->setAddress(I->second); + } + }; + if (auto DVI = dyn_cast(Inst)) + RemapDebugOperands(DVI, DVI->location_ops()); + if (auto DAI = dyn_cast(Inst)) + RemapAssignAddress(DAI); + for (DbgVariableRecord &DVR : filterDbgVars(Inst->getDbgRecordRange())) { + RemapDebugOperands(&DVR, DVR.location_ops()); + if (DVR.isDbgAssign()) + RemapAssignAddress(&DVR); + } + }; + RemapDebugVariable(ValueMapping, New); } return NewBB; diff --git a/llvm/test/Transforms/CallSiteSplitting/callsite-split-debug.ll b/llvm/test/Transforms/CallSiteSplitting/callsite-split-debug.ll index 8f10dcb30d7bf..68c906d616c92 100644 --- a/llvm/test/Transforms/CallSiteSplitting/callsite-split-debug.ll +++ b/llvm/test/Transforms/CallSiteSplitting/callsite-split-debug.ll @@ -1,4 +1,4 @@ -; RUN: opt -S -passes=callsite-splitting -o - < %s | FileCheck %s +; RUN: opt -S -passes=callsite-splitting -o - < %s | FileCheck %s --check-prefixes=CHECK,CHECK-DEBUG ; RUN: opt -S -strip-debug -passes=callsite-splitting -o - < %s | FileCheck %s define internal i16 @bar(i16 %p1, i16 %p2) { @@ -8,6 +8,9 @@ define internal i16 @bar(i16 %p1, i16 %p2) { define i16 @foo(i16 %in) { bb0: + %a = alloca i16, align 4, !DIAssignID !12 + call void @llvm.dbg.assign(metadata i1 undef, metadata !11, metadata !DIExpression(), metadata !12, metadata ptr %a, metadata !DIExpression()), !dbg !8 + store i16 7, ptr %a, align 4, !DIAssignID !13 br label %bb1 bb1: @@ -20,13 +23,21 @@ bb2: CallsiteBB: %1 = phi i16 [ 0, %bb1 ], [ 1, %bb2 ] %c = phi i16 [ 2, %bb1 ], [ 3, %bb2 ] + %p = phi ptr [ %a, %bb1 ], [ %a, %bb2 ] + call void @llvm.dbg.value(metadata i16 %1, metadata !7, metadata !DIExpression()), !dbg !8 call void @llvm.dbg.value(metadata i16 %c, metadata !7, metadata !DIExpression()), !dbg !8 + call void @llvm.dbg.value(metadata !DIArgList(i16 %1, i16 %c), metadata !7, metadata !DIExpression()), !dbg !8 + call void @llvm.dbg.value(metadata !DIArgList(i16 %c, i16 %c), metadata !7, metadata !DIExpression()), !dbg !8 + call void @llvm.dbg.assign(metadata i16 %1, metadata !11, metadata !DIExpression(), metadata !13, metadata ptr %a, metadata !DIExpression()), !dbg !8 + call void @llvm.dbg.assign(metadata i16 %c, metadata !11, metadata !DIExpression(), metadata !13, metadata ptr %a, metadata !DIExpression()), !dbg !8 + call void @llvm.dbg.assign(metadata i16 %1, metadata !11, metadata !DIExpression(), metadata !13, metadata ptr %p, metadata !DIExpression()), !dbg !8 %2 = call i16 @bar(i16 %1, i16 5) ret i16 %2 } ; Function Attrs: nounwind readnone speculatable declare void @llvm.dbg.value(metadata, metadata, metadata) #0 +declare void @llvm.dbg.assign(metadata, metadata, metadata, metadata, metadata, metadata) attributes #0 = { nounwind readnone speculatable } @@ -43,14 +54,37 @@ attributes #0 = { nounwind readnone speculatable } !6 = distinct !DISubprogram(name: "foo", scope: !1, file: !1, line: 4, unit: !0) !7 = !DILocalVariable(name: "c", scope: !6, line: 5, type: !5) !8 = !DILocation(line: 5, column: 7, scope: !6) +!11 = !DILocalVariable(name: "a", scope: !6, line: 6, type: !5) +!12 = distinct !DIAssignID() +!13 = distinct !DIAssignID() ; The optimization should trigger even in the presence of the dbg.value in ; CallSiteBB. ; CHECK-LABEL: @foo ; CHECK-LABEL: bb1.split: +; CHECK-DEBUG: call void @llvm.dbg.value(metadata i16 0, metadata ![[DBG_1:[0-9]+]], {{.*}} +; CHECK-DEBUG: call void @llvm.dbg.value(metadata i16 2, metadata ![[DBG_1]], {{.*}} +; CHECK-DEBUG: call void @llvm.dbg.value(metadata !DIArgList(i16 0, i16 2), {{.*}} +; CHECK-DEBUG: call void @llvm.dbg.value(metadata !DIArgList(i16 2, i16 2), {{.*}} +; CHECK-DEBUG: call void @llvm.dbg.assign(metadata i16 0, metadata ![[DBG_2:[0-9]+]], {{.*}} +; CHECK-DEBUG: call void @llvm.dbg.assign(metadata i16 2, metadata ![[DBG_2]], {{.*}} +; CHECK-DEBUG: call void @llvm.dbg.assign(metadata i16 0, metadata ![[DBG_2]], metadata !DIExpression(), metadata ![[ID_1:[0-9]+]], metadata ptr %a, {{.*}} ; CHECK: [[TMP1:%[0-9]+]] = call i16 @bar(i16 0, i16 5) + ; CHECK-LABEL: bb2.split: +; CHECK-DEBUG: call void @llvm.dbg.value(metadata i16 1, metadata ![[DBG_1]], {{.*}} +; CHECK-DEBUG: call void @llvm.dbg.value(metadata i16 3, metadata ![[DBG_1]], {{.*}} +; CHECK-DEBUG: call void @llvm.dbg.value(metadata !DIArgList(i16 1, i16 3), {{.*}} +; CHECK-DEBUG: call void @llvm.dbg.value(metadata !DIArgList(i16 3, i16 3), {{.*}} +; CHECK-DEBUG: call void @llvm.dbg.assign(metadata i16 1, metadata ![[DBG_2]], {{.*}} +; CHECK-DEBUG: call void @llvm.dbg.assign(metadata i16 3, metadata ![[DBG_2]], {{.*}} +; CHECK-DEBUG: call void @llvm.dbg.assign(metadata i16 1, metadata ![[DBG_2]], metadata !DIExpression(), metadata ![[ID_1:[0-9]+]], metadata ptr %a, {{.*}} ; CHECK: [[TMP2:%[0-9]+]] = call i16 @bar(i16 1, i16 5) + ; CHECK-LABEL: CallsiteBB ; CHECK: %phi.call = phi i16 [ [[TMP2]], %bb2.split ], [ [[TMP1]], %bb1.split + +; CHECK-DEBUG-DAG: ![[DBG_1]] = !DILocalVariable(name: "c"{{.*}}) +; CHECK-DEBUG-DAG: ![[DBG_2]] = !DILocalVariable(name: "a"{{.*}}) +; CHECK-DEBUG-DAG: ![[ID_1]] = distinct !DIAssignID() From 21c102bbe103f56b6cc6843961ca5b18c17de91d Mon Sep 17 00:00:00 2001 From: Carlos Alberto Enciso <47597242+CarlosAlbertoEnciso@users.noreply.github.com> Date: Tue, 9 Apr 2024 13:01:44 +0100 Subject: [PATCH 2/5] [Transforms] Debug values are not remapped when cloning. When cloning instructions from one basic block to another, the debug values are not remapped, in the same was as the normal instructions. Missing condition '|| DbgAssignAddrReplaced' to account for any 'dbg.assign' replacement. --- llvm/lib/IR/IntrinsicInst.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/llvm/lib/IR/IntrinsicInst.cpp b/llvm/lib/IR/IntrinsicInst.cpp index cff716a843099..f7d360f4a453d 100644 --- a/llvm/lib/IR/IntrinsicInst.cpp +++ b/llvm/lib/IR/IntrinsicInst.cpp @@ -137,7 +137,7 @@ void DbgVariableIntrinsic::replaceVariableLocationOp(Value *OldValue, auto Locations = location_ops(); auto OldIt = find(Locations, OldValue); if (OldIt == Locations.end()) { - if (AllowEmpty) + if (AllowEmpty || DbgAssignAddrReplaced) return; assert(DbgAssignAddrReplaced && "OldValue must be dbg.assign addr if unused in DIArgList"); From 8dc837644f8be26ab810d400bb9bf94c4432ca6b Mon Sep 17 00:00:00 2001 From: Carlos Alberto Enciso <47597242+CarlosAlbertoEnciso@users.noreply.github.com> Date: Tue, 16 Apr 2024 13:22:00 +0100 Subject: [PATCH 3/5] [Transforms] Debug values are not remapped when cloning. When cloning instructions from one basic block to another, the debug values are not remapped, in the same was as the normal instructions. - Make 'RemapDebugVariable' a general function by moving it to 'Transform/Utils/Local'. - Change in 'JumpThreading' parameters, return values and local instances: DenseMap to ValueToValueMapTy. --- .../llvm/Transforms/Scalar/JumpThreading.h | 10 +-- llvm/include/llvm/Transforms/Utils/Local.h | 5 ++ llvm/lib/Transforms/Scalar/JumpThreading.cpp | 66 ++++++------------- llvm/lib/Transforms/Utils/CloneFunction.cpp | 28 +------- llvm/lib/Transforms/Utils/Local.cpp | 27 ++++++++ 5 files changed, 59 insertions(+), 77 deletions(-) diff --git a/llvm/include/llvm/Transforms/Scalar/JumpThreading.h b/llvm/include/llvm/Transforms/Scalar/JumpThreading.h index 3364d7eaee424..f7358ac9b1ee0 100644 --- a/llvm/include/llvm/Transforms/Scalar/JumpThreading.h +++ b/llvm/include/llvm/Transforms/Scalar/JumpThreading.h @@ -22,6 +22,7 @@ #include "llvm/Analysis/BranchProbabilityInfo.h" #include "llvm/Analysis/DomTreeUpdater.h" #include "llvm/IR/ValueHandle.h" +#include "llvm/Transforms/Utils/ValueMapper.h" #include #include @@ -114,11 +115,10 @@ class JumpThreadingPass : public PassInfoMixin { bool processBlock(BasicBlock *BB); bool maybeMergeBasicBlockIntoOnlyPred(BasicBlock *BB); void updateSSA(BasicBlock *BB, BasicBlock *NewBB, - DenseMap &ValueMapping); - DenseMap cloneInstructions(BasicBlock::iterator BI, - BasicBlock::iterator BE, - BasicBlock *NewBB, - BasicBlock *PredBB); + ValueToValueMapTy &ValueMapping); + void cloneInstructions(ValueToValueMapTy &ValueMapping, + BasicBlock::iterator BI, BasicBlock::iterator BE, + BasicBlock *NewBB, BasicBlock *PredBB); bool tryThreadEdge(BasicBlock *BB, const SmallVectorImpl &PredBBs, BasicBlock *SuccBB); diff --git a/llvm/include/llvm/Transforms/Utils/Local.h b/llvm/include/llvm/Transforms/Utils/Local.h index 9ae026fa95d21..3487790b395fa 100644 --- a/llvm/include/llvm/Transforms/Utils/Local.h +++ b/llvm/include/llvm/Transforms/Utils/Local.h @@ -18,6 +18,7 @@ #include "llvm/IR/Dominators.h" #include "llvm/Support/CommandLine.h" #include "llvm/Transforms/Utils/SimplifyCFGOptions.h" +#include "llvm/Transforms/Utils/ValueMapper.h" #include namespace llvm { @@ -478,6 +479,10 @@ void hoistAllInstructionsInto(BasicBlock *DomBlock, Instruction *InsertPt, DIExpression *getExpressionForConstant(DIBuilder &DIB, const Constant &C, Type &Ty); +/// Remap the debug intrinsic instructions in the \p Mapping using the +/// \p Inst as a criteria. +void remapDebugVariable(ValueToValueMapTy &Mapping, Instruction *Inst); + //===----------------------------------------------------------------------===// // Intrinsic pattern matching // diff --git a/llvm/lib/Transforms/Scalar/JumpThreading.cpp b/llvm/lib/Transforms/Scalar/JumpThreading.cpp index d7936969011e6..08d82fa66da30 100644 --- a/llvm/lib/Transforms/Scalar/JumpThreading.cpp +++ b/llvm/lib/Transforms/Scalar/JumpThreading.cpp @@ -1876,7 +1876,7 @@ bool JumpThreadingPass::processBranchOnXOR(BinaryOperator *BO) { static void addPHINodeEntriesForMappedBlock(BasicBlock *PHIBB, BasicBlock *OldPred, BasicBlock *NewPred, - DenseMap &ValueMap) { + ValueToValueMapTy &ValueMap) { for (PHINode &PN : PHIBB->phis()) { // Ok, we have a PHI node. Figure out what the incoming value was for the // DestBlock. @@ -1884,7 +1884,7 @@ static void addPHINodeEntriesForMappedBlock(BasicBlock *PHIBB, // Remap the value if necessary. if (Instruction *Inst = dyn_cast(IV)) { - DenseMap::iterator I = ValueMap.find(Inst); + ValueToValueMapTy::iterator I = ValueMap.find(Inst); if (I != ValueMap.end()) IV = I->second; } @@ -1945,9 +1945,8 @@ bool JumpThreadingPass::maybeMergeBasicBlockIntoOnlyPred(BasicBlock *BB) { /// Update the SSA form. NewBB contains instructions that are copied from BB. /// ValueMapping maps old values in BB to new ones in NewBB. -void JumpThreadingPass::updateSSA( - BasicBlock *BB, BasicBlock *NewBB, - DenseMap &ValueMapping) { +void JumpThreadingPass::updateSSA(BasicBlock *BB, BasicBlock *NewBB, + ValueToValueMapTy &ValueMapping) { // If there were values defined in BB that are used outside the block, then we // now have to update all uses of the value to use either the original value, // the cloned value, or some PHI derived value. This can require arbitrary @@ -2008,14 +2007,15 @@ void JumpThreadingPass::updateSSA( /// Clone instructions in range [BI, BE) to NewBB. For PHI nodes, we only clone /// arguments that come from PredBB. Return the map from the variables in the /// source basic block to the variables in the newly created basic block. -DenseMap -JumpThreadingPass::cloneInstructions(BasicBlock::iterator BI, - BasicBlock::iterator BE, BasicBlock *NewBB, - BasicBlock *PredBB) { + +void JumpThreadingPass::cloneInstructions(ValueToValueMapTy &ValueMapping, + BasicBlock::iterator BI, + BasicBlock::iterator BE, + BasicBlock *NewBB, + BasicBlock *PredBB) { // We are going to have to map operands from the source basic block to the new // copy of the block 'NewBB'. If there are PHI nodes in the source basic // block, evaluate them to account for entry from PredBB. - DenseMap ValueMapping; // Retargets llvm.dbg.value to any renamed variables. auto RetargetDbgValueIfPossible = [&](Instruction *NewInst) -> bool { @@ -2103,7 +2103,7 @@ JumpThreadingPass::cloneInstructions(BasicBlock::iterator BI, // Remap operands to patch up intra-block references. for (unsigned i = 0, e = New->getNumOperands(); i != e; ++i) if (Instruction *Inst = dyn_cast(New->getOperand(i))) { - DenseMap::iterator I = ValueMapping.find(Inst); + ValueToValueMapTy::iterator I = ValueMapping.find(Inst); if (I != ValueMapping.end()) New->setOperand(i, I->second); } @@ -2120,7 +2120,7 @@ JumpThreadingPass::cloneInstructions(BasicBlock::iterator BI, RetargetDbgVariableRecordIfPossible(&DVR); } - return ValueMapping; + return; } /// Attempt to thread through two successive basic blocks. @@ -2295,8 +2295,9 @@ void JumpThreadingPass::threadThroughTwoBasicBlocks(BasicBlock *PredPredBB, // We are going to have to map operands from the original BB block to the new // copy of the block 'NewBB'. If there are PHI nodes in PredBB, evaluate them // to account for entry from PredPredBB. - DenseMap ValueMapping = - cloneInstructions(PredBB->begin(), PredBB->end(), NewBB, PredPredBB); + ValueToValueMapTy ValueMapping; + cloneInstructions(ValueMapping, PredBB->begin(), PredBB->end(), NewBB, + PredPredBB); // Copy the edge probabilities from PredBB to NewBB. if (BPI) @@ -2419,8 +2420,9 @@ void JumpThreadingPass::threadEdge(BasicBlock *BB, } // Copy all the instructions from BB to NewBB except the terminator. - DenseMap ValueMapping = - cloneInstructions(BB->begin(), std::prev(BB->end()), NewBB, PredBB); + ValueToValueMapTy ValueMapping; + cloneInstructions(ValueMapping, BB->begin(), std::prev(BB->end()), NewBB, + PredBB); // We didn't copy the terminator from BB over to NewBB, because there is now // an unconditional jump to SuccBB. Insert the unconditional jump. @@ -2675,7 +2677,7 @@ bool JumpThreadingPass::duplicateCondBranchOnPHIIntoPred( // We are going to have to map operands from the original BB block into the // PredBB block. Evaluate PHI nodes in BB. - DenseMap ValueMapping; + ValueToValueMapTy ValueMapping; BasicBlock::iterator BI = BB->begin(); for (; PHINode *PN = dyn_cast(BI); ++BI) @@ -2689,39 +2691,13 @@ bool JumpThreadingPass::duplicateCondBranchOnPHIIntoPred( // Remap operands to patch up intra-block references. for (unsigned i = 0, e = New->getNumOperands(); i != e; ++i) if (Instruction *Inst = dyn_cast(New->getOperand(i))) { - DenseMap::iterator I = ValueMapping.find(Inst); + ValueToValueMapTy::iterator I = ValueMapping.find(Inst); if (I != ValueMapping.end()) New->setOperand(i, I->second); } // Remap debug variable operands. - auto RemapDebugVariable = [](auto &Mapping, auto *Inst) { - auto RemapDebugOperands = [&Mapping](auto *DV, auto Set) { - for (auto *Op : Set) - if (Instruction *InstOp = dyn_cast(Op)) { - auto I = Mapping.find(InstOp); - if (I != Mapping.end()) - DV->replaceVariableLocationOp(Op, I->second, /*AllowEmpty=*/true); - } - }; - auto RemapAssignAddress = [&Mapping](auto *DA) { - if (Instruction *Addr = dyn_cast(DA->getAddress())) { - auto I = Mapping.find(Addr); - if (I != Mapping.end()) - DA->setAddress(I->second); - } - }; - if (auto DVI = dyn_cast(Inst)) - RemapDebugOperands(DVI, DVI->location_ops()); - if (auto DAI = dyn_cast(Inst)) - RemapAssignAddress(DAI); - for (DbgVariableRecord &DVR : filterDbgVars(Inst->getDbgRecordRange())) { - RemapDebugOperands(&DVR, DVR.location_ops()); - if (DVR.isDbgAssign()) - RemapAssignAddress(&DVR); - } - }; - RemapDebugVariable(ValueMapping, New); + remapDebugVariable(ValueMapping, New); // If this instruction can be simplified after the operands are updated, // just use the simplified value instead. This frequently happens due to diff --git a/llvm/lib/Transforms/Utils/CloneFunction.cpp b/llvm/lib/Transforms/Utils/CloneFunction.cpp index de58e1b759f96..303a09805a9d8 100644 --- a/llvm/lib/Transforms/Utils/CloneFunction.cpp +++ b/llvm/lib/Transforms/Utils/CloneFunction.cpp @@ -1133,33 +1133,7 @@ BasicBlock *llvm::DuplicateInstructionsInSplitBetween( } // Remap debug variable operands. - auto RemapDebugVariable = [](auto &Mapping, auto *Inst) { - auto RemapDebugOperands = [&Mapping](auto *DV, auto Set) { - for (auto *Op : Set) - if (Instruction *InstOp = dyn_cast(Op)) { - auto I = Mapping.find(InstOp); - if (I != Mapping.end()) - DV->replaceVariableLocationOp(Op, I->second, /*AllowEmpty=*/true); - } - }; - auto RemapAssignAddress = [&Mapping](auto *DA) { - if (Instruction *Addr = dyn_cast(DA->getAddress())) { - auto I = Mapping.find(Addr); - if (I != Mapping.end()) - DA->setAddress(I->second); - } - }; - if (auto DVI = dyn_cast(Inst)) - RemapDebugOperands(DVI, DVI->location_ops()); - if (auto DAI = dyn_cast(Inst)) - RemapAssignAddress(DAI); - for (DbgVariableRecord &DVR : filterDbgVars(Inst->getDbgRecordRange())) { - RemapDebugOperands(&DVR, DVR.location_ops()); - if (DVR.isDbgAssign()) - RemapAssignAddress(&DVR); - } - }; - RemapDebugVariable(ValueMapping, New); + remapDebugVariable(ValueMapping, New); } return NewBB; diff --git a/llvm/lib/Transforms/Utils/Local.cpp b/llvm/lib/Transforms/Utils/Local.cpp index 380bac9c61807..24731057d27d7 100644 --- a/llvm/lib/Transforms/Utils/Local.cpp +++ b/llvm/lib/Transforms/Utils/Local.cpp @@ -3648,6 +3648,33 @@ DIExpression *llvm::getExpressionForConstant(DIBuilder &DIB, const Constant &C, return nullptr; } +void llvm::remapDebugVariable(ValueToValueMapTy &Mapping, Instruction *Inst) { + auto RemapDebugOperands = [&Mapping](auto *DV, auto Set) { + for (auto *Op : Set) + if (Instruction *InstOp = dyn_cast(Op)) { + auto I = Mapping.find(InstOp); + if (I != Mapping.end()) + DV->replaceVariableLocationOp(Op, I->second, /*AllowEmpty=*/true); + } + }; + auto RemapAssignAddress = [&Mapping](auto *DA) { + if (Instruction *Addr = dyn_cast(DA->getAddress())) { + auto I = Mapping.find(Addr); + if (I != Mapping.end()) + DA->setAddress(I->second); + } + }; + if (auto DVI = dyn_cast(Inst)) + RemapDebugOperands(DVI, DVI->location_ops()); + if (auto DAI = dyn_cast(Inst)) + RemapAssignAddress(DAI); + for (DbgVariableRecord &DVR : filterDbgVars(Inst->getDbgRecordRange())) { + RemapDebugOperands(&DVR, DVR.location_ops()); + if (DVR.isDbgAssign()) + RemapAssignAddress(&DVR); + } +} + namespace { /// A potential constituent of a bitreverse or bswap expression. See From a935889c4715bdb48c4f8007e3259e8a194b447c Mon Sep 17 00:00:00 2001 From: Carlos Alberto Enciso Date: Thu, 25 Apr 2024 15:03:55 +0100 Subject: [PATCH 4/5] [Transforms] Debug values are not remapped when cloning. When cloning instructions from one basic block to another, the debug values are not remapped, in the same was as the normal instructions. - Rewording a comment to add more clarity. - Remove not required 'dyn_cast. --- llvm/include/llvm/Transforms/Utils/Local.h | 4 ++-- llvm/lib/Transforms/Utils/Local.cpp | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/llvm/include/llvm/Transforms/Utils/Local.h b/llvm/include/llvm/Transforms/Utils/Local.h index 3487790b395fa..51be9714ad35c 100644 --- a/llvm/include/llvm/Transforms/Utils/Local.h +++ b/llvm/include/llvm/Transforms/Utils/Local.h @@ -479,8 +479,8 @@ void hoistAllInstructionsInto(BasicBlock *DomBlock, Instruction *InsertPt, DIExpression *getExpressionForConstant(DIBuilder &DIB, const Constant &C, Type &Ty); -/// Remap the debug intrinsic instructions in the \p Mapping using the -/// \p Inst as a criteria. +/// Remap the operands of the debug records attached to \p Inst, and the +/// operands of \p Inst itself if it's a debug intrinsic. void remapDebugVariable(ValueToValueMapTy &Mapping, Instruction *Inst); //===----------------------------------------------------------------------===// diff --git a/llvm/lib/Transforms/Utils/Local.cpp b/llvm/lib/Transforms/Utils/Local.cpp index 24731057d27d7..2dbdf4918e608 100644 --- a/llvm/lib/Transforms/Utils/Local.cpp +++ b/llvm/lib/Transforms/Utils/Local.cpp @@ -3651,7 +3651,7 @@ DIExpression *llvm::getExpressionForConstant(DIBuilder &DIB, const Constant &C, void llvm::remapDebugVariable(ValueToValueMapTy &Mapping, Instruction *Inst) { auto RemapDebugOperands = [&Mapping](auto *DV, auto Set) { for (auto *Op : Set) - if (Instruction *InstOp = dyn_cast(Op)) { + if (auto *InstOp = Op) { auto I = Mapping.find(InstOp); if (I != Mapping.end()) DV->replaceVariableLocationOp(Op, I->second, /*AllowEmpty=*/true); From bdcc5d577f28247a9dfb6f0a7301c9cb338ddeab Mon Sep 17 00:00:00 2001 From: Carlos Alberto Enciso Date: Fri, 26 Apr 2024 06:08:02 +0100 Subject: [PATCH 5/5] [Transforms] Debug values are not remapped when cloning. When cloning instructions from one basic block to another, the debug values are not remapped, in the same was as the normal instructions. - After removing the 'dyn_cast' checks, the following 'if's are not required. --- llvm/lib/Transforms/Utils/Local.cpp | 19 ++++++++----------- 1 file changed, 8 insertions(+), 11 deletions(-) diff --git a/llvm/lib/Transforms/Utils/Local.cpp b/llvm/lib/Transforms/Utils/Local.cpp index 2dbdf4918e608..dfacf433c2684 100644 --- a/llvm/lib/Transforms/Utils/Local.cpp +++ b/llvm/lib/Transforms/Utils/Local.cpp @@ -3650,20 +3650,17 @@ DIExpression *llvm::getExpressionForConstant(DIBuilder &DIB, const Constant &C, void llvm::remapDebugVariable(ValueToValueMapTy &Mapping, Instruction *Inst) { auto RemapDebugOperands = [&Mapping](auto *DV, auto Set) { - for (auto *Op : Set) - if (auto *InstOp = Op) { - auto I = Mapping.find(InstOp); - if (I != Mapping.end()) - DV->replaceVariableLocationOp(Op, I->second, /*AllowEmpty=*/true); - } - }; - auto RemapAssignAddress = [&Mapping](auto *DA) { - if (Instruction *Addr = dyn_cast(DA->getAddress())) { - auto I = Mapping.find(Addr); + for (auto *Op : Set) { + auto I = Mapping.find(Op); if (I != Mapping.end()) - DA->setAddress(I->second); + DV->replaceVariableLocationOp(Op, I->second, /*AllowEmpty=*/true); } }; + auto RemapAssignAddress = [&Mapping](auto *DA) { + auto I = Mapping.find(DA->getAddress()); + if (I != Mapping.end()) + DA->setAddress(I->second); + }; if (auto DVI = dyn_cast(Inst)) RemapDebugOperands(DVI, DVI->location_ops()); if (auto DAI = dyn_cast(Inst))