[llvm-commits] [llvm] r140327 - in /llvm/trunk: lib/Transforms/Scalar/ScalarReplAggregates.cpp test/Transforms/ScalarRepl/2011-09-22-PHISpeculateInvoke.ll

[Eli Friedman]

Author: efriedma
Date: Thu Sep 22 13:56:30 2011
New Revision: 140327

URL: http://llvm.org/viewvc/llvm-project?rev=140327&view=rev
Log:
PR10987: add a missed safety check to isSafePHIToSpeculate in scalarrepl.


Added:
    llvm/trunk/test/Transforms/ScalarRepl/2011-09-22-PHISpeculateInvoke.ll
Modified:
    llvm/trunk/lib/Transforms/Scalar/ScalarReplAggregates.cpp

Modified: llvm/trunk/lib/Transforms/Scalar/ScalarReplAggregates.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Scalar/ScalarReplAggregates.cpp?rev=140327&r1=140326&r2=140327&view=diff
==============================================================================
--- llvm/trunk/lib/Transforms/Scalar/ScalarReplAggregates.cpp (original)
+++ llvm/trunk/lib/Transforms/Scalar/ScalarReplAggregates.cpp Thu Sep 22 13:56:30 2011
@@ -1286,17 +1286,21 @@
   // trapping load in the predecessor if it is a critical edge.
   for (unsigned i = 0, e = PN->getNumIncomingValues(); i != e; ++i) {
     BasicBlock *Pred = PN->getIncomingBlock(i);
+    Value *InVal = PN->getIncomingValue(i);
+
+    // If the terminator of the predecessor has side-effects (an invoke),
+    // there is no safe place to put a load in the predecessor.
+    if (Pred->getTerminator()->mayHaveSideEffects())
+      return false;
+
+    // If the value is produced by the terminator of the predecessor
+    // (an invoke), there is no valid place to put a load in the predecessor.
+    if (Pred->getTerminator() == InVal)
+      return false;
 
     // If the predecessor has a single successor, then the edge isn't critical.
     if (Pred->getTerminator()->getNumSuccessors() == 1)
       continue;
-    
-    Value *InVal = PN->getIncomingValue(i);
-    
-    // If the InVal is an invoke in the pred, we can't put a load on the edge.
-    if (InvokeInst *II = dyn_cast<InvokeInst>(InVal))
-      if (II->getParent() == Pred)
-        return false;
 
     // If this pointer is always safe to load, or if we can prove that there is
     // already a load in the block, then we can move the load to the pred block.

Added: llvm/trunk/test/Transforms/ScalarRepl/2011-09-22-PHISpeculateInvoke.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/ScalarRepl/2011-09-22-PHISpeculateInvoke.ll?rev=140327&view=auto
==============================================================================
--- llvm/trunk/test/Transforms/ScalarRepl/2011-09-22-PHISpeculateInvoke.ll (added)
+++ llvm/trunk/test/Transforms/ScalarRepl/2011-09-22-PHISpeculateInvoke.ll Thu Sep 22 13:56:30 2011
@@ -0,0 +1,40 @@
+; RUN: opt < %s -scalarrepl -S | FileCheck %s
+; PR10987
+
+; Make sure scalarrepl doesn't move a load across an invoke which could
+; modify the loaded value.
+; (The PHI could theoretically be transformed by splitting the critical
+; edge, but scalarrepl doesn't modify the CFG, at least at the moment.)
+
+declare void @extern_fn(i32*)
+declare i32 @extern_fn2(i32)
+declare i32 @__gcc_personality_v0(i32, i64, i8*, i8*)
+
+define void @odd_fn(i1) noinline {
+  %retptr1 = alloca i32
+  %retptr2 = alloca i32
+  br i1 %0, label %then, label %else
+
+then:                                             ; preds = %2
+  invoke void @extern_fn(i32* %retptr1)
+          to label %join unwind label %unwind
+
+else:                                             ; preds = %2
+  store i32 3, i32* %retptr2
+  br label %join
+
+join:                                             ; preds = %then, %else
+  %storemerge.in = phi i32* [ %retptr2, %else ], [ %retptr1, %then ]
+  %storemerge = load i32* %storemerge.in
+  %x3 = call i32 @extern_fn2(i32 %storemerge)
+  ret void
+
+unwind:                                           ; preds = %then
+  %info = landingpad { i8*, i32 } personality i32 (i32, i64, i8*, i8*)* @__gcc_personality_v0
+          cleanup
+  call void @extern_fn(i32* null)
+  unreachable
+}
+
+; CHECK: define void @odd_fn
+; CHECK: %storemerge.in = phi i32* [ %retptr2, %else ], [ %retptr1, %then ]