Ignore debug info when making optimization decisions in SimplifyCFG.

[danielcdh]

Summary

Debug info should *not* affect code generation. This patch properly handles debug info to make sure the generated code are the same with or without debug info.

[jmolloy]

lib/Transforms/Utils/SimplifyCFG.cpp

1493

You shouldn't have to check this here, because canSinkLastInstruction should have checked it.

Sidenote: I think you need to make sure it's checked in canSinkLastInstruction :)

[danielcdh]

lib/Transforms/Utils/SimplifyCFG.cpp

1493

But canSinkLastInstruction only skips the DbgInfoIntrinsic. If we don't have the logic here, I0 may end up being a DbgInfoIntrinsic?

[jmolloy]

Sorry this took so long. LGTM.

lib/Transforms/Utils/SimplifyCFG.cpp

1493

Ah yes, you're right.

[danielcdh]

rebase

lib/Transforms/Utils/SimplifyCFG.cpp

 // instruction of every block in Blocks to their common successor, commoning
 // into one instruction.
 static bool sinkLastInstruction(ArrayRef<BasicBlock*> Blocks) {
   auto *BBEnd = Blocks[0]->getTerminator()->getSuccessor(0);
 
   // canSinkLastInstruction returning true guarantees that every block has at
   // least one non-terminator instruction.
   SmallVector<Instruction*,4> Insts;
-  for (auto *BB : Blocks)
-    Insts.push_back(BB->getTerminator()->getPrevNode());
+  for (auto *BB : Blocks) {
+    Instruction *I = BB->getTerminator();
+    do {
+      I = I->getPrevNode();
+    } while (isa<DbgInfoIntrinsic>(I) && I != &BB->front());
+    if (!isa<DbgInfoIntrinsic>(I))
+      Insts.push_back(I);
+  }
 
   // The only checking we need to do now is that all users of all instructions
   // are the same PHI node. canSinkLastInstruction should have checked this but
   // it is slightly over-aggressive - it gets confused by commutative instructions
   // so double-check it here.
   Instruction *I0 = Insts.front();
   if (!isa<StoreInst>(I0)) {
     auto *PNUse = dyn_cast<PHINode>(*I0->user_begin());
LockstepReverseIterator(ArrayRef<BasicBlock*> Blocks) :
       Blocks(Blocks) {
       reset();
     }
 
     void reset() {
       Fail = false;
       Insts.clear();
       for (auto *BB : Blocks) {
-        if (Instruction *Terminator = BB->getTerminator()) {
-          if (Instruction *LastNonTerminator = Terminator->getPrevNode()) {
-            Insts.push_back(LastNonTerminator);
-            continue;
-          }
-        }
+        Instruction *Inst = BB->getTerminator();
+        for (Inst = Inst->getPrevNode(); Inst && isa<DbgInfoIntrinsic>(Inst);)
+          Inst = Inst->getPrevNode();
+        if (!Inst) {
           // Block wasn't big enough.
           Fail = true;
           return;
         }
+        Insts.push_back(Inst);
+      }
     }
 
     bool isValid() const {
       return !Fail;
     }
     
     void operator -- () {
       if (Fail)
         return;
       for (auto *&Inst : Insts) {
+        for (Inst = Inst->getPrevNode(); Inst && isa<DbgInfoIntrinsic>(Inst);)
           Inst = Inst->getPrevNode();
         // Already at beginning of block.
         if (!Inst) {
           Fail = true;
           return;
         }
       }
     }
 

test/Transforms/SimplifyCFG/sink-common-code.ll

 !1 = !{ !"int", !0 }
 !2 = !{ !"float", !0 }
 !3 = !{ !"const float", !2, i64 0 }
 !4 = !{ !"special float", !2, i64 1 }
 
 ; CHECK-LABEL: test13
 ; CHECK-DAG: select
 ; CHECK-DAG: load volatile
-; CHECK: store volatile {{.*}}, !tbaa !0
+; CHECK: store volatile {{.*}}, !tbaa ![[TBAA:[0-9]]]
 ; CHECK-NOT: load
 ; CHECK-NOT: store
 
 ; The call should be commoned.
 define i32 @test13a(i1 zeroext %flag, i32 %w, i32 %x, i32 %y) {
 entry:
   br i1 %flag, label %if.then, label %if.else
 
if.then:
   %cmp1 = icmp eq i32 %sv1, 56
   br label %if.end
 
 if.else:
   %dummy2 = add i32 %x, 4
   %gepb = getelementptr inbounds %struct.anon, %struct.anon* %s, i32 0, i32 1
   %sv2 = load i32, i32* %gepb
   %cmp2 = icmp eq i32 %sv2, 57
+  call void @llvm.dbg.value(metadata i32 0, i64 0, metadata !9, metadata !DIExpression()), !dbg !11
   br label %if.end
 
 if.end:
   %p = phi i1 [ %cmp1, %if.then ], [ %cmp2, %if.else ]
   ret i32 1
 }
 
+declare void @llvm.dbg.value(metadata, i64, metadata, metadata)
+!llvm.module.flags = !{!5, !6}
+!llvm.dbg.cu = !{!7}
+
+!5 = !{i32 2, !"Dwarf Version", i32 4}
+!6 = !{i32 2, !"Debug Info Version", i32 3}
+!7 = distinct !DICompileUnit(language: DW_LANG_C99, file: !10)
+!8 = distinct !DISubprogram(name: "foo", unit: !7)
+!9 = !DILocalVariable(name: "b", line: 1, arg: 2, scope: !8)
+!10 = !DIFile(filename: "a.c", directory: "a/b")
+!11 = !DILocation(line: 1, column: 14, scope: !8)
+
 ; CHECK-LABEL: test14
 ; CHECK: getelementptr
 ; CHECK: load
 ; CHECK-NOT: load
 
 ; The load should be commoned.
 define i32 @test15(i1 zeroext %flag, i32 %w, i32 %x, i32 %y, %struct.anon* %s) {
 entry:

 }
 
 ; CHECK-LABEL: @test_operand_bundles(
 ; CHECK: left:
 ; CHECK-NEXT:   %val0 = call i32 @call_target() [ "deopt"(i32 10) ]
 ; CHECK: right:
 ; CHECK-NEXT:   %val1 = call i32 @call_target() [ "deopt"(i32 20) ]
 
-; CHECK: !0 = !{!1, !1, i64 0}
-; CHECK: !1 = !{!"float", !2}
-; CHECK: !2 = !{!"an example type tree"}
+; CHECK: ![[TBAA]] = !{![[TYPE:[0-9]]], ![[TYPE]], i64 0}
+; CHECK: ![[TYPE]] = !{!"float", ![[TEXT:[0-9]]]}
+; CHECK: ![[TEXT]] = !{!"an example type tree"}