summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorEli Friedman <eli.friedman@gmail.com>2011-09-22 18:56:30 +0000
committerEli Friedman <eli.friedman@gmail.com>2011-09-22 18:56:30 +0000
commitd102a03b36fb522899cefc31a396c9793b929cf6 (patch)
tree193b5081b0e6fa1c4cb40b4abf15d5f590ff40b0
parent5d8f43fddad852673f2c1fb9543500b542516cd1 (diff)
downloadllvm-d102a03b36fb522899cefc31a396c9793b929cf6.tar.gz
llvm-d102a03b36fb522899cefc31a396c9793b929cf6.tar.bz2
llvm-d102a03b36fb522899cefc31a396c9793b929cf6.tar.xz
PR10987: add a missed safety check to isSafePHIToSpeculate in scalarrepl.
git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@140327 91177308-0d34-0410-b5e6-96231b3b80d8
-rw-r--r--lib/Transforms/Scalar/ScalarReplAggregates.cpp18
-rw-r--r--test/Transforms/ScalarRepl/2011-09-22-PHISpeculateInvoke.ll40
2 files changed, 51 insertions, 7 deletions
diff --git a/lib/Transforms/Scalar/ScalarReplAggregates.cpp b/lib/Transforms/Scalar/ScalarReplAggregates.cpp
index 3721807933..82364c36e4 100644
--- a/lib/Transforms/Scalar/ScalarReplAggregates.cpp
+++ b/lib/Transforms/Scalar/ScalarReplAggregates.cpp
@@ -1286,17 +1286,21 @@ static bool isSafePHIToSpeculate(PHINode *PN, const TargetData *TD) {
// 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.
diff --git a/test/Transforms/ScalarRepl/2011-09-22-PHISpeculateInvoke.ll b/test/Transforms/ScalarRepl/2011-09-22-PHISpeculateInvoke.ll
new file mode 100644
index 0000000000..f98f3e8fc4
--- /dev/null
+++ b/test/Transforms/ScalarRepl/2011-09-22-PHISpeculateInvoke.ll
@@ -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 ]