[llvm] 9649c20 - [InstCombine] Drop debug loc in TryToSinkInstruction (reland)

[Vedant Kumar via llvm-commits]

Author: Vedant Kumar
Date: 2020-06-26T17:18:15-07:00
New Revision: 9649c2095f07a392bc2b2a93b5bd6c4c9bf5ba34

URL: https://github.com/llvm/llvm-project/commit/9649c2095f07a392bc2b2a93b5bd6c4c9bf5ba34
DIFF: https://github.com/llvm/llvm-project/commit/9649c2095f07a392bc2b2a93b5bd6c4c9bf5ba34.diff

LOG: [InstCombine] Drop debug loc in TryToSinkInstruction (reland)

Summary:
The advice in HowToUpdateDebugInfo.rst is to "... preserve the debug
location of an instruction if the instruction either remains in its
basic block, or if its basic block is folded into a predecessor that
branches unconditionally".

TryToSinkInstruction doesn't seem to satisfy the criteria as it's
sinking an instruction to some successor block. Preserving the debug loc
can make single-stepping appear to go backwards, or make a breakpoint
hit on that location happen "too late" (since single-stepping from that
breakpoint can cause the function to return unexpectedly).

So, drop the debug location.

This was reverted in ee3620643dfc because it removed source locations
from inlinable calls, breaking a verifier rule. I've added an exception
for calls because the alternative (setting a line 0 location) is not
better. I tested the updated patch by completing a stage2 RelWithDebInfo
build.

Reviewers: aprantl, davide

Subscribers: hiraditya, llvm-commits

Tags: #llvm

Differential Revision: https://reviews.llvm.org/D82487

Added: 
    llvm/test/Transforms/InstCombine/sink_to_unreachable_dbg.ll

Modified: 
    llvm/lib/Transforms/InstCombine/InstructionCombining.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
index 1f97f0c1ac99..6a406314f63c 100644
--- a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
@@ -3355,6 +3355,12 @@ static bool TryToSinkInstruction(Instruction *I, BasicBlock *DestBlock) {
   I->moveBefore(&*InsertPos);
   ++NumSunkInst;
 
+  // Drop the debug loc of non-inlinable instructions. This prevents
+  // single-stepping from going backwards. See HowToUpdateDebugInfo.rst for
+  // the full rationale.
+  if (!isa<CallBase>(I))
+    I->setDebugLoc(DebugLoc());
+
   // Also sink all related debug uses from the source basic block. Otherwise we
   // get debug use before the def. Attempt to salvage debug uses first, to
   // maximise the range variables have location for. If we cannot salvage, then

diff  --git a/llvm/test/Transforms/InstCombine/sink_to_unreachable_dbg.ll b/llvm/test/Transforms/InstCombine/sink_to_unreachable_dbg.ll
new file mode 100644
index 000000000000..e642276224b8
--- /dev/null
+++ b/llvm/test/Transforms/InstCombine/sink_to_unreachable_dbg.ll
@@ -0,0 +1,46 @@
+; RUN: opt -debugify -debugify-level=locations -instcombine -S < %s | FileCheck %s
+
+; CHECK-LABEL: @test1(
+; CHECK: [[phi:%.*]] = phi i32
+; CHECK-NEXT: [[add:%.*]] = add i32 {{.*}}, 1{{$}}
+; CHECK-NEXT: add i32 [[phi]], [[add]], !dbg
+define i32 @test1(i32 %0, i1 %1) {
+  %3 = add i32 %0, 1
+  br i1 %1, label %4, label %5
+
+4:                                                ; preds = %2
+  br label %6
+
+5:                                                ; preds = %2
+  br label %6
+
+6:                                                ; preds = %5, %4
+  %7 = phi i32 [ 0, %4 ], [ 1, %5 ]
+  %8 = add i32 %7, %3
+  ret i32 %8
+}
+
+; Function Attrs: nounwind readnone
+declare i32 @external(i32) #0
+
+; CHECK-LABEL: @test2(
+; CHECK: [[phi:%.*]] = phi i32
+; CHECK-NEXT: [[add:%.*]] = call i32 @external(i32 {{.*}}), !dbg
+; CHECK-NEXT: add i32 [[phi]], [[add]], !dbg
+define i32 @test2(i32 %0, i1 %1) {
+  %3 = call i32 @external(i32 %0)
+  br i1 %1, label %4, label %5
+
+4:                                                ; preds = %2
+  br label %6
+
+5:                                                ; preds = %2
+  br label %6
+
+6:                                                ; preds = %5, %4
+  %7 = phi i32 [ 0, %4 ], [ 1, %5 ]
+  %8 = add i32 %7, %3
+  ret i32 %8
+}
+
+attributes #0 = { nounwind readnone }