Skip to content

Commit

Permalink
[Transforms] Debug values are not remapped when cloning. (#87747)
Browse files Browse the repository at this point in the history
When cloning instructions from one basic block to another,
the debug values are not remapped, in the same was as the
normal instructions.
  • Loading branch information
CarlosAlbertoEnciso committed Apr 26, 2024
1 parent c4c9d4f commit 7696d36
Show file tree
Hide file tree
Showing 8 changed files with 101 additions and 26 deletions.
3 changes: 2 additions & 1 deletion llvm/include/llvm/IR/IntrinsicInst.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
10 changes: 5 additions & 5 deletions llvm/include/llvm/Transforms/Scalar/JumpThreading.h
Original file line number Diff line number Diff line change
Expand Up @@ -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 <optional>
#include <utility>

Expand Down Expand Up @@ -114,11 +115,10 @@ class JumpThreadingPass : public PassInfoMixin<JumpThreadingPass> {
bool processBlock(BasicBlock *BB);
bool maybeMergeBasicBlockIntoOnlyPred(BasicBlock *BB);
void updateSSA(BasicBlock *BB, BasicBlock *NewBB,
DenseMap<Instruction *, Value *> &ValueMapping);
DenseMap<Instruction *, Value *> 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<BasicBlock *> &PredBBs,
BasicBlock *SuccBB);
Expand Down
5 changes: 5 additions & 0 deletions llvm/include/llvm/Transforms/Utils/Local.h
Original file line number Diff line number Diff line change
Expand Up @@ -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 <cstdint>

namespace llvm {
Expand Down Expand Up @@ -490,6 +491,10 @@ void hoistAllInstructionsInto(BasicBlock *DomBlock, Instruction *InsertPt,
DIExpression *getExpressionForConstant(DIBuilder &DIB, const Constant &C,
Type &Ty);

/// 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);

//===----------------------------------------------------------------------===//
// Intrinsic pattern matching
//
Expand Down
5 changes: 4 additions & 1 deletion llvm/lib/IR/IntrinsicInst.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -136,6 +137,8 @@ void DbgVariableIntrinsic::replaceVariableLocationOp(Value *OldValue,
auto Locations = location_ops();
auto OldIt = find(Locations, OldValue);
if (OldIt == Locations.end()) {
if (AllowEmpty || DbgAssignAddrReplaced)
return;
assert(DbgAssignAddrReplaced &&
"OldValue must be dbg.assign addr if unused in DIArgList");
return;
Expand Down
41 changes: 23 additions & 18 deletions llvm/lib/Transforms/Scalar/JumpThreading.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1876,15 +1876,15 @@ bool JumpThreadingPass::processBranchOnXOR(BinaryOperator *BO) {
static void addPHINodeEntriesForMappedBlock(BasicBlock *PHIBB,
BasicBlock *OldPred,
BasicBlock *NewPred,
DenseMap<Instruction*, Value*> &ValueMap) {
ValueToValueMapTy &ValueMap) {
for (PHINode &PN : PHIBB->phis()) {
// Ok, we have a PHI node. Figure out what the incoming value was for the
// DestBlock.
Value *IV = PN.getIncomingValueForBlock(OldPred);

// Remap the value if necessary.
if (Instruction *Inst = dyn_cast<Instruction>(IV)) {
DenseMap<Instruction*, Value*>::iterator I = ValueMap.find(Inst);
ValueToValueMapTy::iterator I = ValueMap.find(Inst);
if (I != ValueMap.end())
IV = I->second;
}
Expand Down Expand Up @@ -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<Instruction *, Value *> &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
Expand Down Expand Up @@ -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<Instruction *, Value *>
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<Instruction *, Value *> ValueMapping;

// Retargets llvm.dbg.value to any renamed variables.
auto RetargetDbgValueIfPossible = [&](Instruction *NewInst) -> bool {
Expand Down Expand Up @@ -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<Instruction>(New->getOperand(i))) {
DenseMap<Instruction *, Value *>::iterator I = ValueMapping.find(Inst);
ValueToValueMapTy::iterator I = ValueMapping.find(Inst);
if (I != ValueMapping.end())
New->setOperand(i, I->second);
}
Expand All @@ -2120,7 +2120,7 @@ JumpThreadingPass::cloneInstructions(BasicBlock::iterator BI,
RetargetDbgVariableRecordIfPossible(&DVR);
}

return ValueMapping;
return;
}

/// Attempt to thread through two successive basic blocks.
Expand Down Expand Up @@ -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<Instruction *, Value *> 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)
Expand Down Expand Up @@ -2419,8 +2420,9 @@ void JumpThreadingPass::threadEdge(BasicBlock *BB,
}

// Copy all the instructions from BB to NewBB except the terminator.
DenseMap<Instruction *, Value *> 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.
Expand Down Expand Up @@ -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<Instruction*, Value*> ValueMapping;
ValueToValueMapTy ValueMapping;

BasicBlock::iterator BI = BB->begin();
for (; PHINode *PN = dyn_cast<PHINode>(BI); ++BI)
Expand All @@ -2689,11 +2691,14 @@ 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<Instruction>(New->getOperand(i))) {
DenseMap<Instruction*, Value*>::iterator I = ValueMapping.find(Inst);
ValueToValueMapTy::iterator I = ValueMapping.find(Inst);
if (I != ValueMapping.end())
New->setOperand(i, I->second);
}

// Remap debug variable operands.
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.
Expand Down
3 changes: 3 additions & 0 deletions llvm/lib/Transforms/Utils/CloneFunction.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1131,6 +1131,9 @@ BasicBlock *llvm::DuplicateInstructionsInSplitBetween(
if (I != ValueMapping.end())
New->setOperand(i, I->second);
}

// Remap debug variable operands.
remapDebugVariable(ValueMapping, New);
}

return NewBB;
Expand Down
24 changes: 24 additions & 0 deletions llvm/lib/Transforms/Utils/Local.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3685,6 +3685,30 @@ 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) {
auto I = Mapping.find(Op);
if (I != Mapping.end())
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<DbgVariableIntrinsic>(Inst))
RemapDebugOperands(DVI, DVI->location_ops());
if (auto DAI = dyn_cast<DbgAssignIntrinsic>(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
Expand Down
36 changes: 35 additions & 1 deletion llvm/test/Transforms/CallSiteSplitting/callsite-split-debug.ll
Original file line number Diff line number Diff line change
@@ -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) {
Expand All @@ -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:
Expand All @@ -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 }

Expand All @@ -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()

0 comments on commit 7696d36

Please sign in to comment.