diff --git a/lib/Transforms/Scalar/ScalarReplAggregates.cpp b/lib/Transforms/Scalar/ScalarReplAggregates.cpp index 37218079338..82364c36e42 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(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 00000000000..f98f3e8fc45 --- /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 ]