diff --git a/llvm/include/llvm/IR/Instruction.h b/llvm/include/llvm/IR/Instruction.h --- a/llvm/include/llvm/IR/Instruction.h +++ b/llvm/include/llvm/IR/Instruction.h @@ -492,6 +492,20 @@ /// merged DebugLoc. void applyMergedLocation(const DILocation *LocA, const DILocation *LocB); + /// Updates the debug location given that the instruction has been hoisted + /// from a block to a predecessor of that block. + /// Note: it is undefined behavior to call this on an instruction not + /// currently inserted into a function. + void updateLocationAfterHoist(); + + /// Drop the instruction's debug location. This does not guarantee removal + /// of the !dbg source location attachment, as it must set a line 0 location + /// with scope information attached on call instructions. To guarantee + /// removal of the !dbg attachment, use the \ref setDebugLoc() API. + /// Note: it is undefined behavior to call this on an instruction not + /// currently inserted into a function. + void dropLocation(); + private: /// Return true if we have an entry in the on-the-side metadata hash. bool hasMetadataHashEntry() const { diff --git a/llvm/lib/IR/DebugInfo.cpp b/llvm/lib/IR/DebugInfo.cpp --- a/llvm/lib/IR/DebugInfo.cpp +++ b/llvm/lib/IR/DebugInfo.cpp @@ -696,6 +696,34 @@ setDebugLoc(DILocation::getMergedLocation(LocA, LocB)); } +void Instruction::updateLocationAfterHoist() { dropLocation(); } + +void Instruction::dropLocation() { + const DebugLoc &DL = getDebugLoc(); + if (!DL) + return; + + // If this isn't a call, drop the location to allow a location from a + // preceding instruction to propagate. + if (!isa(this)) { + setDebugLoc(DebugLoc()); + return; + } + + // Set a line 0 location for calls to preserve scope information in case + // inlining occurs. + const DISubprogram *SP = getFunction()->getSubprogram(); + if (SP) + // If a function scope is available, set it on the line 0 location. When + // hoisting a call to a predecessor block, using the function scope avoids + // making it look like the callee was reached earlier than it should be. + setDebugLoc(DebugLoc::get(0, 0, SP)); + else + // The parent function has no scope. Absent a better alternative, create a + // line 0 location with the existing scope/inlinedAt info. + setDebugLoc(DebugLoc::get(0, 0, DL.getScope(), DL.getInlinedAt())); +} + //===----------------------------------------------------------------------===// // LLVM C API implementations. //===----------------------------------------------------------------------===// diff --git a/llvm/lib/Transforms/Scalar/GVN.cpp b/llvm/lib/Transforms/Scalar/GVN.cpp --- a/llvm/lib/Transforms/Scalar/GVN.cpp +++ b/llvm/lib/Transforms/Scalar/GVN.cpp @@ -48,7 +48,6 @@ #include "llvm/IR/Constant.h" #include "llvm/IR/Constants.h" #include "llvm/IR/DataLayout.h" -#include "llvm/IR/DebugInfoMetadata.h" #include "llvm/IR/DebugLoc.h" #include "llvm/IR/Dominators.h" #include "llvm/IR/Function.h" @@ -1320,8 +1319,7 @@ // Instructions that have been inserted in predecessor(s) to materialize // the load address do not retain their original debug locations. Doing // so could lead to confusing (but correct) source attributions. - if (const DebugLoc &DL = I->getDebugLoc()) - I->setDebugLoc(DebugLoc::get(0, 0, DL.getScope(), DL.getInlinedAt())); + I->updateLocationAfterHoist(); // FIXME: We really _ought_ to insert these value numbers into their // parent's availability map. However, in doing so, we risk getting into diff --git a/llvm/lib/Transforms/Scalar/LICM.cpp b/llvm/lib/Transforms/Scalar/LICM.cpp --- a/llvm/lib/Transforms/Scalar/LICM.cpp +++ b/llvm/lib/Transforms/Scalar/LICM.cpp @@ -1658,10 +1658,7 @@ // Move the new node to the destination block, before its terminator. moveInstructionBefore(I, *Dest->getTerminator(), *SafetyInfo, MSSAU, SE); - // Apply line 0 debug locations when we are moving instructions to different - // basic blocks because we want to avoid jumpy line tables. - if (const DebugLoc &DL = I.getDebugLoc()) - I.setDebugLoc(DebugLoc::get(0, 0, DL.getScope(), DL.getInlinedAt())); + I.updateLocationAfterHoist(); if (isa(I)) ++NumMovedLoads; diff --git a/llvm/test/DebugInfo/Generic/licm-hoist-debug-loc.ll b/llvm/test/DebugInfo/Generic/licm-hoist-debug-loc.ll --- a/llvm/test/DebugInfo/Generic/licm-hoist-debug-loc.ll +++ b/llvm/test/DebugInfo/Generic/licm-hoist-debug-loc.ll @@ -18,9 +18,8 @@ ; We make sure that the instruction that is hoisted into the preheader ; does not have a debug location. ; CHECK: for.body.lr.ph: -; CHECK: getelementptr{{.*}}%p.addr, i64 4{{.*}} !dbg [[zero:![0-9]+]] +; CHECK: getelementptr{{.*}}%p.addr, i64 4{{$}} ; CHECK: for.body: -; CHECK: [[zero]] = !DILocation(line: 0 ; ; ModuleID = 't.ll' source_filename = "test.c" diff --git a/llvm/test/Transforms/GVN/PRE/phi-translate.ll b/llvm/test/Transforms/GVN/PRE/phi-translate.ll --- a/llvm/test/Transforms/GVN/PRE/phi-translate.ll +++ b/llvm/test/Transforms/GVN/PRE/phi-translate.ll @@ -4,8 +4,8 @@ ; CHECK-LABEL: @foo( ; CHECK: entry.end_crit_edge: -; CHECK: %[[INDEX:[a-z0-9.]+]] = sext i32 %x to i64{{.*}} !dbg [[ZERO_LOC:![0-9]+]] -; CHECK: %[[ADDRESS:[a-z0-9.]+]] = getelementptr [100 x i32], [100 x i32]* @G, i64 0, i64 %[[INDEX]]{{.*}} !dbg [[ZERO_LOC]] +; CHECK: %[[INDEX:[a-z0-9.]+]] = sext i32 %x to i64{{$}} +; CHECK: %[[ADDRESS:[a-z0-9.]+]] = getelementptr [100 x i32], [100 x i32]* @G, i64 0, i64 %[[INDEX]]{{$}} ; CHECK: %n.pre = load i32, i32* %[[ADDRESS]], align 4, !dbg [[N_LOC:![0-9]+]] ; CHECK: br label %end ; CHECK: then: @@ -14,8 +14,7 @@ ; CHECK: %n = phi i32 [ %n.pre, %entry.end_crit_edge ], [ %z, %then ], !dbg [[N_LOC]] ; CHECK: ret i32 %n -; CHECK-DAG: [[N_LOC]] = !DILocation(line: 47, column: 1, scope: !{{.*}}) -; CHECK-DAG: [[ZERO_LOC]] = !DILocation(line: 0 +; CHECK: [[N_LOC]] = !DILocation(line: 47, column: 1, scope: !{{.*}}) @G = external global [100 x i32] define i32 @foo(i32 %x, i32 %z) !dbg !6 { diff --git a/llvm/test/Transforms/LICM/hoisting-preheader-debugloc.ll b/llvm/test/Transforms/LICM/hoisting-preheader-debugloc.ll --- a/llvm/test/Transforms/LICM/hoisting-preheader-debugloc.ll +++ b/llvm/test/Transforms/LICM/hoisting-preheader-debugloc.ll @@ -1,6 +1,6 @@ ; RUN: opt -S -licm < %s | FileCheck %s -; CHECK: %arrayidx4.promoted = load i32, i32* %arrayidx4, align 1, !tbaa !59 +; CHECK: %arrayidx4.promoted = load i32, i32* %arrayidx4, align 1, !tbaa !{{[0-9]+$}} target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128" target triple = "x86_64-unknown-linux-gnu" diff --git a/llvm/unittests/IR/InstructionsTest.cpp b/llvm/unittests/IR/InstructionsTest.cpp --- a/llvm/unittests/IR/InstructionsTest.cpp +++ b/llvm/unittests/IR/InstructionsTest.cpp @@ -13,6 +13,7 @@ #include "llvm/IR/BasicBlock.h" #include "llvm/IR/Constants.h" #include "llvm/IR/DataLayout.h" +#include "llvm/IR/DebugInfoMetadata.h" #include "llvm/IR/DerivedTypes.h" #include "llvm/IR/Function.h" #include "llvm/IR/IRBuilder.h" @@ -1304,5 +1305,80 @@ I->deleteValue(); } +TEST(InstructionsTest, UpdateLocationAfterHoist) { + LLVMContext C; + std::unique_ptr M = parseIR(C, + R"( + declare void @callee() + + define void @no_parent_scope() { + call void @callee() ; I1: Call with no location. + call void @callee(), !dbg !11 ; I2: Call with location. + ret void, !dbg !11 ; I3: Non-call with location. + } + + define void @with_parent_scope() !dbg !8 { + call void @callee() ; I1: Call with no location. + call void @callee(), !dbg !11 ; I2: Call with location. + ret void, !dbg !11 ; I3: Non-call with location. + } + + !llvm.dbg.cu = !{!0} + !llvm.module.flags = !{!3, !4} + !0 = distinct !DICompileUnit(language: DW_LANG_C99, file: !1, producer: "clang version 6.0.0", isOptimized: false, runtimeVersion: 0, emissionKind: FullDebug, enums: !2) + !1 = !DIFile(filename: "t2.c", directory: "foo") + !2 = !{} + !3 = !{i32 2, !"Dwarf Version", i32 4} + !4 = !{i32 2, !"Debug Info Version", i32 3} + !8 = distinct !DISubprogram(name: "f", scope: !1, file: !1, line: 1, type: !9, isLocal: false, isDefinition: true, scopeLine: 1, isOptimized: false, unit: !0, retainedNodes: !2) + !9 = !DISubroutineType(types: !10) + !10 = !{null} + !11 = !DILocation(line: 2, column: 7, scope: !8, inlinedAt: !12) + !12 = !DILocation(line: 3, column: 8, scope: !8) + )"); + ASSERT_TRUE(M); + + { + Function *NoParentScopeF = + cast(M->getNamedValue("no_parent_scope")); + BasicBlock &BB = NoParentScopeF->front(); + + auto *I1 = BB.getFirstNonPHI(); + auto *I2 = I1->getNextNode(); + auto *I3 = BB.getTerminator(); + + EXPECT_FALSE(bool(I1->getDebugLoc())); + I1->dropLocation(); + EXPECT_FALSE(bool(I1->getDebugLoc())); + + const MDNode *Scope = I2->getDebugLoc().getScope(); + const DILocation *InlinedAt = I2->getDebugLoc().getInlinedAt(); + EXPECT_EQ(I2->getDebugLoc().getLine(), 2U); + I2->dropLocation(); + EXPECT_EQ(I2->getDebugLoc().getLine(), 0U); + EXPECT_EQ(I2->getDebugLoc().getScope(), Scope); + EXPECT_EQ(I2->getDebugLoc().getInlinedAt(), InlinedAt); + + EXPECT_EQ(I3->getDebugLoc().getLine(), 2U); + I3->dropLocation(); + EXPECT_FALSE(bool(I3->getDebugLoc())); + } + + { + Function *WithParentScopeF = + cast(M->getNamedValue("with_parent_scope")); + BasicBlock &BB = WithParentScopeF->front(); + + auto *I2 = BB.getFirstNonPHI()->getNextNode(); + + MDNode *Scope = cast(WithParentScopeF->getSubprogram()); + EXPECT_EQ(I2->getDebugLoc().getLine(), 2U); + I2->dropLocation(); + EXPECT_EQ(I2->getDebugLoc().getLine(), 0U); + EXPECT_EQ(I2->getDebugLoc().getScope(), Scope); + EXPECT_EQ(I2->getDebugLoc().getInlinedAt(), nullptr); + } +} + } // end anonymous namespace } // end namespace llvm