[SimplifyCFG] Drop debug loc in SpeculativelyExecuteBB

[vsk]

Summary

According to HowToUpdateDebugInfo.rst:

Preserving the debug locations of speculated instructions can make
it seem like a condition is true when it's not (or vice versa), which
leads to a confusing single-stepping experience

This patch follows the recommendation to drop debug locations on
speculated instructions.

[vsk]

Back out the change for speculated store instructions: a merged location is applied to this instruction.

[davide]

Yes, this makes a lot of sense.

[davide]

do we have an end-to-end testcase where this triggers?

llvm/lib/Transforms/Utils/SimplifyCFG.cpp

2149

I prefer DebugLoc() but up to you.

[vsk]

The issue triggers quite often in a stage2 build: I count 50K instances and the build is less than half complete.

Here's a synthetic example. Before the patch, lldb single-steps to line 7 after visiting line 4:

frame #0: 0x0000000100000f6a t`main(argc=1, argv=0x00007ffeefbff8e8) at t.cc:7:3 [opt]
   4      int y = x;
   5      if (y) {
   6        y += 9;
-> 7      }
   8      y -= 11;
   9      printf("%d\n", y);
   10     return 0;

llvm/lib/Transforms/Utils/SimplifyCFG.cpp

2149

I'll fix this before committing.

llvm/lib/Transforms/Utils/SimplifyCFG.cpp

if (SpeculatedStoreValue) {
     Value *S = Builder.CreateSelect(
         BrCond, TrueV, FalseV, "spec.store.select", BI);
     SpeculatedStore->setOperand(0, S);
     SpeculatedStore->applyMergedLocation(BI->getDebugLoc(),
                                          SpeculatedStore->getDebugLoc());
   }
 
   // Metadata can be dependent on the condition we are hoisting above.
-  // Conservatively strip all metadata on the instruction.
-  for (auto &I : *ThenBB)
+  // Conservatively strip all metadata on the instruction. Drop the debug loc
+  // to avoid making it appear as if the condition is a constant, which would
+  // be misleading while debugging.
+  for (auto &I : *ThenBB) {
+    if (!SpeculatedStoreValue || &I != SpeculatedStore)
+      I.setDebugLoc(DebugLoc());
     I.dropUnknownNonDebugMetadata();
+  }
 
   // Hoist the instructions.
   BB->getInstList().splice(BI->getIterator(), ThenBB->getInstList(),
                            ThenBB->begin(), std::prev(ThenBB->end()));
 
   // Insert selects and rewrite the PHI operands.
   IRBuilder<NoFolder> Builder(BI);
   for (PHINode &PN : EndBB->phis()) {

llvm/test/Transforms/SimplifyCFG/drop-debug-loc-when-speculating.ll

+; RUN: opt -S -o - %s -simplifycfg | FileCheck %s
+
+declare i1 @make_condition()
+
+; CHECK-LABEL: @test1(
+; CHECK: and i1 [[COND:%.*]], [[COND]]{{$}}
+; CHECK: select i1 [[COND]]
+define void @test1() !dbg !6 {
+start:
+  %cond = call i1 @make_condition(), !dbg !8
+  br i1 %cond, label %then, label %else, !dbg !9
+
+then:                                             ; preds = %start
+  %and = and i1 %cond, %cond, !dbg !10
+  br label %else, !dbg !11
+
+else:                                             ; preds = %then, %start
+  %phi = phi i1 [ %cond, %start ], [ %and, %then ], !dbg !12
+  ret void, !dbg !13
+}
+
+!llvm.dbg.cu = !{!0}
+!llvm.debugify = !{!3, !4}
+!llvm.module.flags = !{!5}
+
+!0 = distinct !DICompileUnit(language: DW_LANG_C, file: !1, producer: "debugify", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug, enums: !2)
+!1 = !DIFile(filename: "test.ll", directory: "/")
+!2 = !{}
+!3 = !{i32 6}
+!4 = !{i32 0}
+!5 = !{i32 2, !"Debug Info Version", i32 3}
+!6 = distinct !DISubprogram(name: "test1", linkageName: "test1", scope: null, file: !1, line: 1, type: !7, scopeLine: 1, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: !0, retainedNodes: !2)
+!7 = !DISubroutineType(types: !2)
+!8 = !DILocation(line: 1, column: 1, scope: !6)
+!9 = !DILocation(line: 2, column: 1, scope: !6)
+!10 = !DILocation(line: 3, column: 2, scope: !6)
+!11 = !DILocation(line: 4, column: 2, scope: !6)
+!12 = !DILocation(line: 5, column: 3, scope: !6)
+!13 = !DILocation(line: 6, column: 3, scope: !6)