[llvm] r183152 - SimplifyCFG: Do not transform PHI to select if doing so would be unsafe

[David Majnemer]

Author: majnemer
Date: Mon Jun  3 15:43:12 2013
New Revision: 183152

URL: http://llvm.org/viewvc/llvm-project?rev=183152&view=rev
Log:
SimplifyCFG: Do not transform PHI to select if doing so would be unsafe

PR16069 is an interesting case where an incoming value to a PHI is a
trap value while also being a 'ConstantExpr'.

We do not consider this case when performing the 'HoistThenElseCodeToIf'
optimization.

Instead, make our modifications more conservative if we detect that we
cannot transform the PHI to a select.

Added:
    llvm/trunk/test/Transforms/SimplifyCFG/PR16069.ll
Modified:
    llvm/trunk/lib/Transforms/Utils/SimplifyCFG.cpp

Modified: llvm/trunk/lib/Transforms/Utils/SimplifyCFG.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Utils/SimplifyCFG.cpp?rev=183152&r1=183151&r2=183152&view=diff
==============================================================================
--- llvm/trunk/lib/Transforms/Utils/SimplifyCFG.cpp (original)
+++ llvm/trunk/lib/Transforms/Utils/SimplifyCFG.cpp Mon Jun  3 15:43:12 2013
@@ -1081,9 +1081,9 @@ static bool HoistThenElseCodeToIf(Branch
       (isa<InvokeInst>(I1) && !isSafeToHoistInvoke(BB1, BB2, I1, I2)))
     return false;
 
-  // If we get here, we can hoist at least one instruction.
   BasicBlock *BIParent = BI->getParent();
 
+  bool Changed = false;
   do {
     // If we are hoisting the terminator instruction, don't move one (making a
     // broken BB), instead clone it, and remove BI.
@@ -1098,6 +1098,7 @@ static bool HoistThenElseCodeToIf(Branch
       I2->replaceAllUsesWith(I1);
     I1->intersectOptionalDataWith(I2);
     I2->eraseFromParent();
+    Changed = true;
 
     I1 = BB1_Itr++;
     I2 = BB2_Itr++;
@@ -1117,7 +1118,23 @@ static bool HoistThenElseCodeToIf(Branch
 HoistTerminator:
   // It may not be possible to hoist an invoke.
   if (isa<InvokeInst>(I1) && !isSafeToHoistInvoke(BB1, BB2, I1, I2))
-    return true;
+    return Changed;
+
+  for (succ_iterator SI = succ_begin(BB1), E = succ_end(BB1); SI != E; ++SI) {
+    PHINode *PN;
+    for (BasicBlock::iterator BBI = SI->begin();
+         (PN = dyn_cast<PHINode>(BBI)); ++BBI) {
+      Value *BB1V = PN->getIncomingValueForBlock(BB1);
+      Value *BB2V = PN->getIncomingValueForBlock(BB2);
+      if (BB1V == BB2V)
+        continue;
+
+      if (isa<ConstantExpr>(BB1V) && !isSafeToSpeculativelyExecute(BB1V))
+        return Changed;
+      if (isa<ConstantExpr>(BB2V) && !isSafeToSpeculativelyExecute(BB2V))
+        return Changed;
+    }
+  }
 
   // Okay, it is safe to hoist the terminator.
   Instruction *NT = I1->clone();

Added: llvm/trunk/test/Transforms/SimplifyCFG/PR16069.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/SimplifyCFG/PR16069.ll?rev=183152&view=auto
==============================================================================
--- llvm/trunk/test/Transforms/SimplifyCFG/PR16069.ll (added)
+++ llvm/trunk/test/Transforms/SimplifyCFG/PR16069.ll Mon Jun  3 15:43:12 2013
@@ -0,0 +1,14 @@
+; RUN: opt < %s -simplifycfg -S | FileCheck %s
+
+; CHECK-NOT: select
+ at b = extern_weak global i32
+define i32 @foo(i1 %y) {
+  br i1 %y, label %bb1, label %bb2
+bb1:
+  br label %bb3
+bb2:
+  br label %bb3
+bb3:
+  %cond.i = phi i32 [ 0, %bb1 ], [ srem (i32 1, i32 zext (i1 icmp eq (i32* @b, i32* null) to i32)), %bb2 ]
+  ret i32 %cond.i
+}