[PATCH] D46635: [SimplifyCFG] Fix a debug invariant bug in FoldBranchToCommonDest()

[David Stenberg via Phabricator via llvm-commits]

dstenb created this revision.
dstenb added reviewers: fhahn, craig.topper, dblaikie.
Herald added a subscriber: llvm-commits.

Fix a case where FoldBranchToCommonDest() would bail out from doing CSE
when encountering a debug intrinsic. Handle that by skipping past the
debug intrinsics.

Also, as a minor refactoring, rename checkCSEInPredecessor() to
tryCSEWithPredecessor() to make it a bit more clear that the function
may remove instructions.


Repository:
  rL LLVM

https://reviews.llvm.org/D46635

Files:
  lib/Transforms/Utils/SimplifyCFG.cpp
  test/Transforms/SimplifyCFG/fold-branch-debuginvariant.ll


Index: test/Transforms/SimplifyCFG/fold-branch-debuginvariant.ll
===================================================================
--- /dev/null
+++ test/Transforms/SimplifyCFG/fold-branch-debuginvariant.ll
@@ -0,0 +1,49 @@
+; RUN: opt < %s -simplifycfg -S | FileCheck %s
+; RUN: opt < %s -strip-debug -simplifycfg -S | FileCheck %s
+
+; Verify that the and.2 instruction is eliminated even in the presence of a
+; preceding debug intrinsic.
+
+; CHECK: bb1:
+; CHECK: and i1
+; CHECK: bb2:
+; CHECK-NOT: and i1
+
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+; Function Attrs: norecurse nounwind
+define void @foo() local_unnamed_addr #0 !dbg !4 {
+bb1:
+  %and.1 = and i1 false, false
+  %cmp = icmp eq i16 0, 0
+  br i1 %cmp, label %bb2, label %bb3
+
+bb2:                                              ; preds = %bb1
+  call void @llvm.dbg.value(metadata i16 0, metadata !8, metadata !DIExpression()), !dbg !9
+  %and.2 = and i1 false, false
+  br label %bb3
+
+bb3:                                              ; preds = %bb2, %bb1
+  ret void
+}
+
+; Function Attrs: nounwind readnone speculatable
+declare void @llvm.dbg.value(metadata, metadata, metadata) #1
+
+attributes #0 = { norecurse nounwind }
+attributes #1 = { nounwind readnone speculatable }
+
+!llvm.dbg.cu = !{!0}
+!llvm.module.flags = !{!3}
+
+!0 = distinct !DICompileUnit(language: DW_LANG_C, file: !1, producer: "Foo", isOptimized: false, runtimeVersion: 0, emissionKind: FullDebug, enums: !2, retainedTypes: !2, globals: !2)
+!1 = !DIFile(filename: "foo.c", directory: "/")
+!2 = !{}
+!3 = !{i32 2, !"Debug Info Version", i32 3}
+!4 = distinct !DISubprogram(name: "foo", scope: !1, file: !1, line: 10, type: !5, isLocal: false, isDefinition: true, scopeLine: 10, isOptimized: false, unit: !0, retainedNodes: !2)
+!5 = !DISubroutineType(types: !6)
+!6 = !{null, !7}
+!7 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
+!8 = !DILocalVariable(name: "p_1", arg: 1, scope: !4, line: 4, type: !7)
+!9 = distinct !DILocation(line: 11, column: 3, scope: !4)
Index: lib/Transforms/Utils/SimplifyCFG.cpp
===================================================================
--- lib/Transforms/Utils/SimplifyCFG.cpp
+++ lib/Transforms/Utils/SimplifyCFG.cpp
@@ -2497,7 +2497,7 @@
 
 /// Return true if the given instruction is available
 /// in its predecessor block. If yes, the instruction will be removed.
-static bool checkCSEInPredecessor(Instruction *Inst, BasicBlock *PB) {
+static bool tryCSEWithPredecessor(Instruction *Inst, BasicBlock *PB) {
   if (!isa<BinaryOperator>(Inst) && !isa<CmpInst>(Inst))
     return false;
   for (Instruction &I : *PB) {
@@ -2554,14 +2554,16 @@
         if (PBI->isConditional() &&
             (BI->getSuccessor(0) == PBI->getSuccessor(0) ||
              BI->getSuccessor(0) == PBI->getSuccessor(1))) {
-          for (BasicBlock::iterator I = BB->begin(), E = BB->end(); I != E;) {
+          for (auto I = BB->instructionsWithoutDebug().begin(),
+                    E = BB->instructionsWithoutDebug().end();
+               I != E;) {
             Instruction *Curr = &*I++;
             if (isa<CmpInst>(Curr)) {
               Cond = Curr;
               break;
             }
             // Quit if we can't remove this instruction.
-            if (!checkCSEInPredecessor(Curr, PB))
+            if (!tryCSEWithPredecessor(Curr, PB))
               return false;
           }
         }


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D46635.145895.patch
Type: text/x-patch
Size: 3477 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20180509/703a1be5/attachment.bin>