Skip to content
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

[DebugInfo][RemoveDIs] Support cloning and remapping DPValues #72546

Closed
wants to merge 8 commits into from
22 changes: 22 additions & 0 deletions llvm/include/llvm/Transforms/Utils/ValueMapper.h
Original file line number Diff line number Diff line change
Expand Up @@ -15,17 +15,21 @@
#define LLVM_TRANSFORMS_UTILS_VALUEMAPPER_H

#include "llvm/ADT/ArrayRef.h"
#include "llvm/ADT/simple_ilist.h"
#include "llvm/IR/ValueHandle.h"
#include "llvm/IR/ValueMap.h"

namespace llvm {

class Constant;
class DIBuilder;
class DPValue;
class Function;
class GlobalVariable;
class Instruction;
class MDNode;
class Metadata;
class Module;
class Type;
class Value;

Expand Down Expand Up @@ -175,6 +179,8 @@ class ValueMapper {
Constant *mapConstant(const Constant &C);

void remapInstruction(Instruction &I);
void remapDPValue(Module *M, DPValue &V);
void remapDPValueRange(Module *M, iterator_range<simple_ilist<DPValue>::iterator> Range);
OCHyams marked this conversation as resolved.
Show resolved Hide resolved
void remapFunction(Function &F);
void remapGlobalObjectMetadata(GlobalObject &GO);

Expand Down Expand Up @@ -260,6 +266,22 @@ inline void RemapInstruction(Instruction *I, ValueToValueMapTy &VM,
ValueMapper(VM, Flags, TypeMapper, Materializer).remapInstruction(*I);
}

/// Remap the Values used in the DPValue \a V using the value map \a VM.
inline void RemapDPValue(Module *M, DPValue *V, ValueToValueMapTy &VM,
RemapFlags Flags = RF_None,
ValueMapTypeRemapper *TypeMapper = nullptr,
ValueMaterializer *Materializer = nullptr) {
ValueMapper(VM, Flags, TypeMapper, Materializer).remapDPValue(M, *V);
}

/// Remap the Values used in the DPValue \a V using the value map \a VM.
inline void RemapDPValueRange(Module *M, iterator_range<simple_ilist<DPValue>::iterator> Range, ValueToValueMapTy &VM,
RemapFlags Flags = RF_None,
ValueMapTypeRemapper *TypeMapper = nullptr,
ValueMaterializer *Materializer = nullptr) {
ValueMapper(VM, Flags, TypeMapper, Materializer).remapDPValueRange(M, Range);
}

/// Remap the operands, metadata, arguments, and instructions of a function.
///
/// Calls \a MapValue() on prefix data, prologue data, and personality
Expand Down
2 changes: 2 additions & 0 deletions llvm/lib/Transforms/Scalar/SimpleLoopUnswitch.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1260,8 +1260,10 @@ static BasicBlock *buildClonedLoopBlocks(
// everything available. Also, we have inserted new instructions which may
// include assume intrinsics, so we update the assumption cache while
// processing this.
Module *M = ClonedPH->getParent()->getParent();
for (auto *ClonedBB : NewBlocks)
for (Instruction &I : *ClonedBB) {
RemapDPValueRange(M, I.getDbgValueRange(), VMap, RF_NoModuleLevelChanges | RF_IgnoreMissingLocals);
RemapInstruction(&I, VMap,
RF_NoModuleLevelChanges | RF_IgnoreMissingLocals);
if (auto *II = dyn_cast<AssumeInst>(&I))
Expand Down
5 changes: 4 additions & 1 deletion llvm/lib/Transforms/Utils/CloneFunction.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,10 @@ BasicBlock *llvm::CloneBasicBlock(const BasicBlock *BB, ValueToValueMapTy &VMap,
Instruction *NewInst = I.clone();
if (I.hasName())
NewInst->setName(I.getName() + NameSuffix);
NewInst->insertInto(NewBB, NewBB->end());

NewInst->insertBefore(*NewBB, NewBB->end());
NewInst->cloneDebugInfoFrom(&I);

VMap[&I] = NewInst; // Add instruction map to value.

if (isa<CallInst>(I) && !I.isDebugOrPseudoInst()) {
Expand Down
5 changes: 5 additions & 0 deletions llvm/lib/Transforms/Utils/LoopUnrollRuntime.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -913,9 +913,14 @@ bool llvm::UnrollRuntimeLoopRemainder(
// Rewrite the cloned instruction operands to use the values created when the
// clone is created.
for (BasicBlock *BB : NewBlocks) {
Module *M = BB->getModule();
for (Instruction &I : *BB) {
RemapInstruction(&I, VMap,
RF_NoModuleLevelChanges | RF_IgnoreMissingLocals);
for (DPValue &DPV : I.getDbgValueRange()) {
RemapDPValue(M, &DPV, VMap,
RF_NoModuleLevelChanges | RF_IgnoreMissingLocals);
}
OCHyams marked this conversation as resolved.
Show resolved Hide resolved
}
}

Expand Down
30 changes: 30 additions & 0 deletions llvm/lib/Transforms/Utils/SimplifyCFG.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1126,6 +1126,10 @@ static void CloneInstructionsIntoPredecessorBlockAndUpdateSSAUses(
NewBonusInst->dropUBImplyingAttrsAndMetadata();

NewBonusInst->insertInto(PredBlock, PTI->getIterator());
NewBonusInst->cloneDebugInfoFrom(&BonusInst);

for (DPValue &DPV : NewBonusInst->getDbgValueRange())
RemapDPValue(NewBonusInst->getModule(), &DPV, VMap, RF_NoModuleLevelChanges | RF_IgnoreMissingLocals);

if (isa<DbgInfoIntrinsic>(BonusInst))
continue;
Expand Down Expand Up @@ -3298,6 +3302,10 @@ FoldCondBranchOnValueKnownInPredecessorImpl(BranchInst *BI, DomTreeUpdater *DTU,
BasicBlock::iterator InsertPt = EdgeBB->getFirstInsertionPt();
DenseMap<Value *, Value *> TranslateMap; // Track translated values.
TranslateMap[Cond] = CB;

// RemoveDIs: track instructions that we optimise away while folding, so
// that we can copy DPValues from them later.
BasicBlock::iterator DbgInfoCursor = BB->begin();
for (BasicBlock::iterator BBI = BB->begin(); &*BBI != BI; ++BBI) {
if (PHINode *PN = dyn_cast<PHINode>(BBI)) {
TranslateMap[PN] = PN->getIncomingValueForBlock(EdgeBB);
Expand Down Expand Up @@ -3332,13 +3340,26 @@ FoldCondBranchOnValueKnownInPredecessorImpl(BranchInst *BI, DomTreeUpdater *DTU,
TranslateMap[&*BBI] = N;
}
if (N) {
// Copy all debug-info attached to instructions from the last we
// successfully clone, up to this instruction (they might have been
// folded away).
for (; DbgInfoCursor != BBI; ++DbgInfoCursor)
Copy link
Contributor

Choose a reason for hiding this comment

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

Hasn't eraseFromParent been called on the DbgInfoCursor instructions?

Copy link
Member Author

Choose a reason for hiding this comment

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

It's iterating over the source block where things don't get erased, I think it's only the clones that can be simplified away in the blocks above that get deleted. This is a legitimate confusion though, perhaps renaming it to SrcDbgCursor makes it clearer?

N->cloneDebugInfoFrom(&*DbgInfoCursor);
DbgInfoCursor = std::next(BBI);
// Clone debug-info on this instruction too.
N->cloneDebugInfoFrom(&*BBI);

// Register the new instruction with the assumption cache if necessary.
if (auto *Assume = dyn_cast<AssumeInst>(N))
if (AC)
AC->registerAssumption(Assume);
}
}

for (; &*DbgInfoCursor != BI; ++DbgInfoCursor)
InsertPt->cloneDebugInfoFrom(&*DbgInfoCursor);
InsertPt->cloneDebugInfoFrom(BI);

BB->removePredecessor(EdgeBB);
BranchInst *EdgeBI = cast<BranchInst>(EdgeBB->getTerminator());
EdgeBI->setSuccessor(0, RealDest);
Expand Down Expand Up @@ -3743,6 +3764,15 @@ static bool performBranchToCommonDestFolding(BranchInst *BI, BranchInst *PBI,
ValueToValueMapTy VMap; // maps original values to cloned values
CloneInstructionsIntoPredecessorBlockAndUpdateSSAUses(BB, PredBlock, VMap);

Module *M = BB->getModule();

if (PredBlock->IsNewDbgInfoFormat) {
PredBlock->getTerminator()->cloneDebugInfoFrom(BB->getTerminator());
for (DPValue &DPV : PredBlock->getTerminator()->getDbgValueRange()) {
RemapDPValue(M, &DPV, VMap, RF_NoModuleLevelChanges | RF_IgnoreMissingLocals);
}
}

// Now that the Cond was cloned into the predecessor basic block,
// or/and the two conditions together.
Value *BICond = VMap[BI->getCondition()];
Expand Down
43 changes: 43 additions & 0 deletions llvm/lib/Transforms/Utils/ValueMapper.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
#include "llvm/IR/InlineAsm.h"
#include "llvm/IR/Instruction.h"
#include "llvm/IR/Instructions.h"
#include "llvm/IR/IntrinsicInst.h"
#include "llvm/IR/Metadata.h"
#include "llvm/IR/Operator.h"
#include "llvm/IR/Type.h"
Expand Down Expand Up @@ -145,6 +146,7 @@ class Mapper {
Value *mapValue(const Value *V);
void remapInstruction(Instruction *I);
void remapFunction(Function &F);
void remapDPValue(DPValue &DPV);

Constant *mapConstant(const Constant *C) {
return cast_or_null<Constant>(mapValue(C));
Expand Down Expand Up @@ -535,6 +537,37 @@ Value *Mapper::mapValue(const Value *V) {
return getVM()[V] = ConstantPointerNull::get(cast<PointerType>(NewTy));
}

void Mapper::remapDPValue(DPValue &V) {
// Remap variables and DILocations.
auto *MappedVar = mapMetadata(V.getVariable());
auto *MappedDILoc = mapMetadata(V.getDebugLoc());
V.setVariable(cast<DILocalVariable>(MappedVar));
V.setDebugLoc(DebugLoc(cast<DILocation>(MappedDILoc)));

// Find Value operands and remap those.
SmallVector<Value *, 4> Vals, NewVals;
for (Value *Val: V.location_ops())
Vals.push_back(Val);
for (Value *Val : Vals)
NewVals.push_back(mapValue(Val));

// If there are no changes to the Value operands, finished.
if (Vals == NewVals)
return;

bool IgnoreMissingLocals = Flags & RF_IgnoreMissingLocals;

// Otherwise, do some replacement.
if (!IgnoreMissingLocals &&
llvm::any_of(NewVals, [&](Value *V) { return V == nullptr;})) {
V.setKillLocation();
} else {
for (unsigned int I = 0; I < Vals.size(); ++I)
if (NewVals[I] || !IgnoreMissingLocals)
Copy link
Contributor

Choose a reason for hiding this comment

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

If IgnoreMissingLocals is false then we know none of the NewVals are nullptr because of the outer if check. So we just need the non-null check, without the || !IgnoreMissingLocals (if !IgnoreMissingLocals and value-is-null then we'd have hit the outer if block). Maybe !IgnoreMissingLocals should be an assert after the non-null check?

Copy link
Member Author

Choose a reason for hiding this comment

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

Good catch; I've just removed the check and added a comment, asserting something that's guaranteed three lines above seems like overkill.

V.replaceVariableLocationOp(I, NewVals[I]);
}
}

Value *Mapper::mapBlockAddress(const BlockAddress &BA) {
Function *F = cast<Function>(mapValue(BA.getFunction()));

Expand Down Expand Up @@ -1179,6 +1212,16 @@ void ValueMapper::remapInstruction(Instruction &I) {
FlushingMapper(pImpl)->remapInstruction(&I);
}

void ValueMapper::remapDPValue(Module *M, DPValue &V) {
FlushingMapper(pImpl)->remapDPValue(V);
}

void ValueMapper::remapDPValueRange(Module *M, iterator_range<DPValue::self_iterator> Range) {
for (DPValue &DPV : Range) {
remapDPValue(M, DPV);
}
}

void ValueMapper::remapFunction(Function &F) {
FlushingMapper(pImpl)->remapFunction(F);
}
Expand Down
13 changes: 13 additions & 0 deletions llvm/test/Transforms/LoopUnroll/runtime-epilog-debuginfo.ll
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
; RUN: opt -passes=loop-unroll -unroll-runtime -unroll-runtime-epilog -S %s | FileCheck %s
; RUN: opt -passes=loop-unroll -unroll-runtime -unroll-runtime-epilog -S %s --try-experimental-debuginfo-iterators | FileCheck %s

; Test that epilogue is tagged with the same debug information as original loop body rather than original loop exit.

Expand All @@ -11,6 +12,18 @@
; CHECK: br i1 %lcmp.mod, label %for.body.i.epil.preheader, label %lee1.exit.loopexit, !dbg ![[LOOP_LOC]]
; CHECK: for.body.i.epil.preheader:
; CHECK: br label %for.body.i.epil, !dbg ![[LOOP_LOC]]
; CHECK: for.body.i.epil:
;; Ensure that when we clone the div/add/add and it's following dbg.values,
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: it's -> its

;; those dbg.values are remapped to the duplicated adds, not the originals.
; CHECK: %div.i.epil = sdiv i32 %t.08.i.epil, 2,
; CHECK-NEXT: %add.i.epil = add i32 %t.08.i.epil, %a,
; CHECK-NEXT: %add1.i.epil = add i32 %add.i.epil, %div.i.epil,
; CHECK-NEXT: call void @llvm.dbg.value(metadata i32 %add1.i.epil,
; CHECK-NEXT: %inc.i.epil = add nuw i32 %i.09.i.epil, 1, !dbg !36
; CHECK-NEXT: call void @llvm.dbg.value(metadata i32 %inc.i.epil,
; CHECK-NEXT: call void @llvm.dbg.value(metadata i32 %inc.i.epil,
; CHECK-NEXT: call void @llvm.dbg.value(metadata i32 %add1.i.epil,

; CHECK: lee1.exit.loopexit:
; CHECK: br label %lee1.exit, !dbg ![[EXIT_LOC:[0-9]+]]

Expand Down
108 changes: 108 additions & 0 deletions llvm/test/Transforms/SimpleLoopUnswitch/debuginfo.ll
Original file line number Diff line number Diff line change
@@ -0,0 +1,108 @@
; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
; RUN: opt -passes='loop-mssa(simple-loop-unswitch<nontrivial>),verify<loops>' -S < %s | FileCheck %s
; RUN: opt -passes='loop-mssa(simple-loop-unswitch<nontrivial>),verify<loops>' -S < %s --try-experimental-debuginfo-iterators | FileCheck %s
;
;; Check that when we duplicate the load in the loop header, we also duplicate
;; the corresponding dbg.value.
;; FIXME: the hoisted load dominates the duplicated dbg.value, however as it's
;; not subsequently used in the loop, so it doesn't get remapped into the
;; debug user and we get a undef/poison dbg.value. This is suboptimal, but it's
;; important that it gets duplicated nonetheless.
Copy link
Contributor

Choose a reason for hiding this comment

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

important that it gets duplicated nonetheless.

"it" being "the behaviour"?


declare void @clobber()
declare void @llvm.dbg.value(metadata, metadata, metadata)

define i32 @partial_unswitch_true_successor(ptr %ptr, i32 %N) {
; CHECK-LABEL: @partial_unswitch_true_successor(
; CHECK-NEXT: entry:
; CHECK-NEXT: [[TMP0:%.*]] = load i32, ptr [[PTR:%.*]], align 4
; CHECK-NEXT: [[TMP1:%.*]] = icmp eq i32 [[TMP0]], 100
; CHECK-NEXT: br i1 [[TMP1]], label [[ENTRY_SPLIT_US:%.*]], label [[ENTRY_SPLIT:%.*]]
; CHECK: entry.split.us:
; CHECK-NEXT: br label [[LOOP_HEADER_US:%.*]]
; CHECK: loop.header.us:
; CHECK-NEXT: [[IV_US:%.*]] = phi i32 [ 0, [[ENTRY_SPLIT_US]] ], [ [[IV_NEXT_US:%.*]], [[LOOP_LATCH_US:%.*]] ]
; CHECK-NEXT: call void @llvm.dbg.value(metadata i32 poison, metadata [[META3:![0-9]+]], metadata !DIExpression()), !dbg [[DBG8:![0-9]+]]
; CHECK-NEXT: br label [[NOCLOBBER_US:%.*]]
; CHECK: noclobber.us:
; CHECK-NEXT: br label [[LOOP_LATCH_US]]
; CHECK: loop.latch.us:
; CHECK-NEXT: [[C_US:%.*]] = icmp ult i32 [[IV_US]], [[N:%.*]]
; CHECK-NEXT: [[IV_NEXT_US]] = add i32 [[IV_US]], 1
; CHECK-NEXT: br i1 [[C_US]], label [[LOOP_HEADER_US]], label [[EXIT_SPLIT_US:%.*]]
; CHECK: exit.split.us:
; CHECK-NEXT: br label [[EXIT:%.*]]
; CHECK: entry.split:
; CHECK-NEXT: br label [[LOOP_HEADER:%.*]]
; CHECK: loop.header:
; CHECK-NEXT: [[IV:%.*]] = phi i32 [ 0, [[ENTRY_SPLIT]] ], [ [[IV_NEXT:%.*]], [[LOOP_LATCH:%.*]] ]
; CHECK-NEXT: [[LV:%.*]] = load i32, ptr [[PTR]], align 4
; CHECK-NEXT: call void @llvm.dbg.value(metadata i32 [[LV]], metadata [[META3]], metadata !DIExpression()), !dbg [[DBG8]]
; CHECK-NEXT: [[SC:%.*]] = icmp eq i32 [[LV]], 100
; CHECK-NEXT: br i1 [[SC]], label [[NOCLOBBER:%.*]], label [[CLOBBER:%.*]]
; CHECK: noclobber:
; CHECK-NEXT: br label [[LOOP_LATCH]]
; CHECK: clobber:
; CHECK-NEXT: call void @clobber()
; CHECK-NEXT: br label [[LOOP_LATCH]]
; CHECK: loop.latch:
; CHECK-NEXT: [[C:%.*]] = icmp ult i32 [[IV]], [[N]]
; CHECK-NEXT: [[IV_NEXT]] = add i32 [[IV]], 1
; CHECK-NEXT: br i1 [[C]], label [[LOOP_HEADER]], label [[EXIT_SPLIT:%.*]], !llvm.loop [[LOOP9:![0-9]+]]
; CHECK: exit.split:
; CHECK-NEXT: br label [[EXIT]]
; CHECK: exit:
; CHECK-NEXT: ret i32 10
;
entry:
br label %loop.header

loop.header:
%iv = phi i32 [ 0, %entry ], [ %iv.next, %loop.latch ]
%lv = load i32, ptr %ptr
call void @llvm.dbg.value(metadata i32 %lv, metadata !6, metadata !DIExpression()), !dbg !7
%sc = icmp eq i32 %lv, 100
br i1 %sc, label %noclobber, label %clobber

noclobber:
br label %loop.latch

clobber:
call void @clobber()
br label %loop.latch

loop.latch:
%c = icmp ult i32 %iv, %N
%iv.next = add i32 %iv, 1
br i1 %c, label %loop.header, label %exit

exit:
ret i32 10
}

!llvm.module.flags = !{!21}
!llvm.dbg.cu = !{!2}

!0 = distinct !DISubprogram(name: "foo", line: 2, isLocal: false, isDefinition: true, virtualIndex: 6, flags: DIFlagPrototyped, isOptimized: false, unit: !2, file: !20, scope: !1, type: !3)
!1 = !DIFile(filename: "b.c", directory: "/private/tmp")
!2 = distinct !DICompileUnit(language: DW_LANG_C99, producer: "clang", isOptimized: true, emissionKind: FullDebug, file: !20)
!3 = !DISubroutineType(types: !4)
!4 = !{!5}
!5 = !DIBasicType(tag: DW_TAG_base_type, name: "int", size: 32, align: 32, encoding: DW_ATE_signed)
!6 = !DILocalVariable(name: "i", line: 2, arg: 1, scope: !0, file: !1, type: !5)
!7 = !DILocation(line: 2, column: 13, scope: !0)
!9 = !DILocalVariable(name: "k", line: 3, scope: !10, file: !1, type: !5)
!10 = distinct !DILexicalBlock(line: 2, column: 16, file: !20, scope: !0)
!11 = !DILocation(line: 3, column: 12, scope: !10)
!12 = !DILocation(line: 4, column: 3, scope: !10)
!13 = !DILocation(line: 5, column: 5, scope: !14)
!14 = distinct !DILexicalBlock(line: 4, column: 10, file: !20, scope: !10)
!15 = !DILocation(line: 6, column: 3, scope: !14)
!16 = !DILocation(line: 7, column: 5, scope: !17)
!17 = distinct !DILexicalBlock(line: 6, column: 10, file: !20, scope: !10)
!18 = !DILocation(line: 8, column: 3, scope: !17)
!19 = !DILocation(line: 9, column: 3, scope: !10)
!20 = !DIFile(filename: "b.c", directory: "/private/tmp")
!21 = !{i32 1, !"Debug Info Version", i32 3}


Loading
Loading