From cc245c26cb10a3e171aa976ed17c2557e21bdb9a Mon Sep 17 00:00:00 2001
From: Jakub Kuderski <kubakuderski@gmail.com>
Date: Thu, 26 Sep 2019 15:20:17 +0000
Subject: [PATCH] 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

llvm-svn: 372989
---
 lib/Transforms/Utils/FlattenCFG.cpp | 12 +++++++++++-
 test/Transforms/Util/flattencfg.ll  | 30 +++++++++++++++++++++++++++++
 2 files changed, 41 insertions(+), 1 deletion(-)

diff --git a/lib/Transforms/Utils/FlattenCFG.cpp b/lib/Transforms/Utils/FlattenCFG.cpp
index 41109746de5..893f23eb604 100644
--- a/lib/Transforms/Utils/FlattenCFG.cpp
+++ b/lib/Transforms/Utils/FlattenCFG.cpp
@@ -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(BasicBlock *BB, IRBuilder<> &Builder) {
   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();
diff --git a/test/Transforms/Util/flattencfg.ll b/test/Transforms/Util/flattencfg.ll
index 1814d6e1f8e..3b31476df0b 100644
--- a/test/Transforms/Util/flattencfg.ll
+++ b/test/Transforms/Util/flattencfg.ll
@@ -54,3 +54,33 @@ bb0:                                               ; preds = %bb1, %entry
   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
+}