[llvm] r372989 - Handle successor's PHI node correctly when flattening CFG merges two if-regions

[Jakub Kuderski via llvm-commits]

Author: kuhar
Date: Thu Sep 26 08:20:17 2019
New Revision: 372989

URL: http://llvm.org/viewvc/llvm-project?rev=372989&view=rev
Log:
Handle successor's PHI node correctly when flattening CFG merges two if-regions

Summary:
FlattenCFG merges two 'if' basicblocks by inserting one basicblock
to another basicblock. The inserted basicblock can have a successor
that contains a PHI node whoes incoming basicblock is the inserted
basicblock. Since the existing code does not handle it, it becomes
a badref.

if (cond1)
  statement
if (cond2)
  statement
successor - contains PHI node whose predecessor is cond2

-->
if (cond1 || cond2)
  statement
(BB for cond2 was deleted)
successor - contains PHI node whose predecessor is cond2 --> bad ref!

Author: Jaebaek Seo

Reviewers: asbirlea, kuhar, tstellar, chandlerc, davide, dexonsmith

Reviewed By: kuhar

Subscribers: hiraditya, llvm-commits

Tags: #llvm

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

Modified:
    llvm/trunk/lib/Transforms/Utils/FlattenCFG.cpp
    llvm/trunk/test/Transforms/Util/flattencfg.ll

Modified: llvm/trunk/lib/Transforms/Utils/FlattenCFG.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Utils/FlattenCFG.cpp?rev=372989&r1=372988&r2=372989&view=diff
==============================================================================
--- llvm/trunk/lib/Transforms/Utils/FlattenCFG.cpp (original)
+++ llvm/trunk/lib/Transforms/Utils/FlattenCFG.cpp Thu Sep 26 08:20:17 2019
@@ -67,7 +67,7 @@ public:
 /// Before:
 ///   ......
 ///   %cmp10 = fcmp une float %tmp1, %tmp2
-///   br i1 %cmp1, label %if.then, label %lor.rhs
+///   br i1 %cmp10, label %if.then, label %lor.rhs
 ///
 /// lor.rhs:
 ///   ......
@@ -453,6 +453,16 @@ bool FlattenCFGOpt::MergeIfRegion(BasicB
   PBI->replaceUsesOfWith(CC, NC);
   Builder.SetInsertPoint(SaveInsertBB, SaveInsertPt);
 
+  // Handle PHI node to replace its predecessors to FirstEntryBlock.
+  for (BasicBlock *Succ : successors(PBI)) {
+    for (PHINode &Phi : Succ->phis()) {
+      for (unsigned i = 0, e = Phi.getNumIncomingValues(); i != e; ++i) {
+        if (Phi.getIncomingBlock(i) == SecondEntryBlock)
+          Phi.setIncomingBlock(i, FirstEntryBlock);
+      }
+    }
+  }
+
   // Remove IfTrue1
   if (IfTrue1 != FirstEntryBlock) {
     IfTrue1->dropAllReferences();

Modified: llvm/trunk/test/Transforms/Util/flattencfg.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/Util/flattencfg.ll?rev=372989&r1=372988&r2=372989&view=diff
==============================================================================
--- llvm/trunk/test/Transforms/Util/flattencfg.ll (original)
+++ llvm/trunk/test/Transforms/Util/flattencfg.ll Thu Sep 26 08:20:17 2019
@@ -54,3 +54,33 @@ bb0:
   br i1 %1, label %bb4, label %bb3
 }
 
+; CHECK-LABEL: @test_not_crash3
+; CHECK-NEXT:  entry:
+; CHECK-NEXT:    %a_eq_0 = icmp eq i32 %a, 0
+; CHECK-NEXT:    %a_eq_1 = icmp eq i32 %a, 1
+; CHECK-NEXT:    [[COND:%[a-z0-9]+]] = or i1 %a_eq_0, %a_eq_1
+; CHECK-NEXT:    br i1 [[COND]], label %bb2, label %bb3
+; CHECK:       bb2:
+; CHECK-NEXT:    br label %bb3
+; CHECK:       bb3:
+; CHECK-NEXT:    %check_badref = phi i32 [ 17, %entry ], [ 11, %bb2 ]
+; CHECK-NEXT:    ret void
+define void @test_not_crash3(i32 %a) #0 {
+entry:
+  %a_eq_0 = icmp eq i32 %a, 0
+  br i1 %a_eq_0, label %bb0, label %bb1
+
+bb0:                                              ; preds = %entry
+  br label %bb1
+
+bb1:                                              ; preds = %bb0, %entry
+  %a_eq_1 = icmp eq i32 %a, 1
+  br i1 %a_eq_1, label %bb2, label %bb3
+
+bb2:                                              ; preds = %bb1
+  br label %bb3
+
+bb3:                                              ; preds = %bb2, %bb1
+  %check_badref = phi i32 [ 17, %bb1 ], [ 11, %bb2 ]
+  ret void
+}