[llvm-commits] [llvm] r173340 - in /llvm/trunk: lib/Transforms/Utils/SimplifyCFG.cpp test/Transforms/SimplifyCFG/SpeculativeExec.ll

[Chandler Carruth]

Author: chandlerc
Date: Thu Jan 24 05:53:01 2013
New Revision: 173340

URL: http://llvm.org/viewvc/llvm-project?rev=173340&view=rev
Log:
Switch the constant expression speculation cost evaluation away from
a cost fuction that seems both a bit ad-hoc and also poorly suited to
evaluating constant expressions.

Notably, it is missing any support for trivial expressions such as
'inttoptr'. I could fix this routine, but it isn't clear to me all of
the constraints its other users are operating under.

The core protection that seems relevant here is avoiding the formation
of a select instruction wich a further chain of select operations in
a constant expression operand. Just explicitly encode that constraint.

Also, update the comments and organization here to make it clear where
this needs to go -- this should be driven off of real cost measurements
which take into account the number of constants expressions and the
depth of the constant expression tree.

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

Modified: llvm/trunk/lib/Transforms/Utils/SimplifyCFG.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Utils/SimplifyCFG.cpp?rev=173340&r1=173339&r2=173340&view=diff
==============================================================================
--- llvm/trunk/lib/Transforms/Utils/SimplifyCFG.cpp (original)
+++ llvm/trunk/lib/Transforms/Utils/SimplifyCFG.cpp Thu Jan 24 05:53:01 2013
@@ -1433,21 +1433,28 @@
       continue;
 
     HaveRewritablePHIs = true;
-
-    // Check for safety.
     ConstantExpr *CE = dyn_cast<ConstantExpr>(ThenV);
     if (!CE)
-      continue; // Known safe.
+      continue; // Known safe and cheap.
+
+    if (!isSafeToSpeculativelyExecute(CE))
+      return false;
+
+    // Don't speculate into a select with a constant select expression operand.
+    // FIXME: This should really be a cost metric, but our cost model doesn't
+    // accurately model the expense of select.
+    if (Operator::getOpcode(CE) == Instruction::Select)
+      return false;
 
     // An unfolded ConstantExpr could end up getting expanded into
     // Instructions. Don't speculate this and another instruction at
     // the same time.
+    // FIXME: This is strange because provided we haven't already hit the cost
+    // of 1, this code will speculate an arbitrary number of complex constant
+    // expression PHI nodes. Also, this doesn't account for how complex the
+    // constant expression is.
     if (SpeculationCost > 0)
       return false;
-    if (!isSafeToSpeculativelyExecute(CE))
-      return false;
-    if (ComputeSpeculationCost(CE) > PHINodeFoldingThreshold)
-      return false;
   }
 
   // If there are no PHIs to process, bail early. This helps ensure idempotence

Modified: llvm/trunk/test/Transforms/SimplifyCFG/SpeculativeExec.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/SimplifyCFG/SpeculativeExec.ll?rev=173340&r1=173339&r2=173340&view=diff
==============================================================================
--- llvm/trunk/test/Transforms/SimplifyCFG/SpeculativeExec.ll (original)
+++ llvm/trunk/test/Transforms/SimplifyCFG/SpeculativeExec.ll Thu Jan 24 05:53:01 2013
@@ -44,3 +44,25 @@
   ret i8 %c
 }
 
+define i8* @test3(i1* %dummy, i8* %a, i8* %b) {
+; Test that a weird, unfolded constant cast in the PHI don't block speculation.
+; CHECK: @test3
+
+entry:
+  %cond1 = load volatile i1* %dummy
+  br i1 %cond1, label %if, label %end
+
+if:
+  %cond2 = load volatile i1* %dummy
+  br i1 %cond2, label %then, label %end
+
+then:
+  br label %end
+
+end:
+  %x = phi i8* [ %a, %entry ], [ %b, %if ], [ inttoptr (i64 42 to i8*), %then ]
+; CHECK-NOT: phi
+; CHECK: select i1 %cond2, i8* inttoptr
+
+  ret i8* %x
+}