diff --git a/lib/Transforms/Scalar/GVN.cpp b/lib/Transforms/Scalar/GVN.cpp index 1e89ef7b22d..f4a98984447 100644 --- a/lib/Transforms/Scalar/GVN.cpp +++ b/lib/Transforms/Scalar/GVN.cpp @@ -37,6 +37,7 @@ #include "llvm/Support/Compiler.h" #include "llvm/Support/Debug.h" #include "llvm/Transforms/Utils/BasicBlockUtils.h" +#include "llvm/Transforms/Utils/Local.h" #include using namespace llvm; @@ -1075,6 +1076,7 @@ bool GVN::processNonLocalLoad(LoadInst *LI, BasicBlock *TmpBB = LoadBB; bool isSinglePred = false; + bool allSingleSucc = true; while (TmpBB->getSinglePredecessor()) { isSinglePred = true; TmpBB = TmpBB->getSinglePredecessor(); @@ -1084,6 +1086,8 @@ bool GVN::processNonLocalLoad(LoadInst *LI, return false; if (Blockers.count(TmpBB)) return false; + if (TmpBB->getTerminator()->getNumSuccessors() != 1) + allSingleSucc = false; } assert(TmpBB); @@ -1160,7 +1164,20 @@ bool GVN::processNonLocalLoad(LoadInst *LI, << UnavailablePred->getName() << "': " << *LI); return false; } - + + // Make sure it is valid to move this load here. We have to watch out for: + // @1 = getelementptr (i8* p, ... + // test p and branch if == 0 + // load @1 + // It is valid to have the getelementptr before the test, even if p can be 0, + // as getelementptr only does address arithmetic. + // If we are not pushing the value through any multiple-successor blocks + // we do not have this case. Otherwise, check that the load is safe to + // put anywhere; this can be improved, but should be conservatively safe. + if (!allSingleSucc && + !isSafeToLoadUnconditionally(LoadPtr, UnavailablePred->getTerminator())) + return false; + // Okay, we can eliminate this load by inserting a reload in the predecessor // and using PHI construction to get the value in the other predecessors, do // it. diff --git a/test/Transforms/GVN/2009-06-17-InvalidPRE.ll b/test/Transforms/GVN/2009-06-17-InvalidPRE.ll new file mode 100644 index 00000000000..c8982c86cb9 --- /dev/null +++ b/test/Transforms/GVN/2009-06-17-InvalidPRE.ll @@ -0,0 +1,72 @@ +; RUN: llvm-as < %s | opt -gvn -enable-load-pre | llvm-dis | not grep pre1 +; GVN load pre was hoisting the loads at %13 and %16 up to bb4.outer. +; This is invalid as it bypasses the check for %m.0.ph==null in bb4. +; ModuleID = 'mbuf.c' +target datalayout = "e-p:32:32:32-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:32:64-f32:32:32-f64:32:64-v64:64:64-v128:128:128-a0:0:64-f80:128:128" +target triple = "i386-apple-darwin9.6" + %struct.mbuf = type { %struct.mbuf*, %struct.mbuf*, i32, i8*, i16, i16, i32 } + +define void @m_adj(%struct.mbuf* %mp, i32 %req_len) nounwind optsize { +entry: + %0 = icmp eq %struct.mbuf* %mp, null ; [#uses=1] + %1 = icmp slt i32 %req_len, 0 ; [#uses=1] + %or.cond = or i1 %1, %0 ; [#uses=1] + br i1 %or.cond, label %return, label %bb4.preheader + +bb4.preheader: ; preds = %entry + br label %bb4.outer + +bb2: ; preds = %bb1 + %2 = sub i32 %len.0, %13 ; [#uses=1] + %3 = getelementptr %struct.mbuf* %m.0.ph, i32 0, i32 2 ; [#uses=1] + store i32 0, i32* %3, align 4 + %4 = getelementptr %struct.mbuf* %m.0.ph, i32 0, i32 0 ; <%struct.mbuf**> [#uses=1] + %5 = load %struct.mbuf** %4, align 4 ; <%struct.mbuf*> [#uses=1] + br label %bb4.outer + +bb4.outer: ; preds = %bb4.preheader, %bb2 + %m.0.ph = phi %struct.mbuf* [ %5, %bb2 ], [ %mp, %bb4.preheader ] ; <%struct.mbuf*> [#uses=7] + %len.0.ph = phi i32 [ %2, %bb2 ], [ %req_len, %bb4.preheader ] ; [#uses=1] + %6 = icmp ne %struct.mbuf* %m.0.ph, null ; [#uses=1] + %7 = getelementptr %struct.mbuf* %m.0.ph, i32 0, i32 2 ; [#uses=1] + %8 = getelementptr %struct.mbuf* %m.0.ph, i32 0, i32 2 ; [#uses=1] + %9 = getelementptr %struct.mbuf* %m.0.ph, i32 0, i32 3 ; [#uses=1] + %10 = getelementptr %struct.mbuf* %m.0.ph, i32 0, i32 3 ; [#uses=1] + br label %bb4 + +bb4: ; preds = %bb4.outer, %bb3 + %len.0 = phi i32 [ 0, %bb3 ], [ %len.0.ph, %bb4.outer ] ; [#uses=6] + %11 = icmp sgt i32 %len.0, 0 ; [#uses=1] + %12 = and i1 %11, %6 ; [#uses=1] + br i1 %12, label %bb1, label %bb7 + +bb1: ; preds = %bb4 + %13 = load i32* %7, align 4 ; [#uses=3] + %14 = icmp sgt i32 %13, %len.0 ; [#uses=1] + br i1 %14, label %bb3, label %bb2 + +bb3: ; preds = %bb1 + %15 = sub i32 %13, %len.0 ; [#uses=1] + store i32 %15, i32* %8, align 4 + %16 = load i8** %9, align 4 ; [#uses=1] + %17 = getelementptr i8* %16, i32 %len.0 ; [#uses=1] + store i8* %17, i8** %10, align 4 + br label %bb4 + +bb7: ; preds = %bb4 + %18 = getelementptr %struct.mbuf* %mp, i32 0, i32 5 ; [#uses=1] + %19 = load i16* %18, align 2 ; [#uses=1] + %20 = zext i16 %19 to i32 ; [#uses=1] + %21 = and i32 %20, 2 ; [#uses=1] + %22 = icmp eq i32 %21, 0 ; [#uses=1] + br i1 %22, label %return, label %bb8 + +bb8: ; preds = %bb7 + %23 = sub i32 %req_len, %len.0 ; [#uses=1] + %24 = getelementptr %struct.mbuf* %mp, i32 0, i32 6 ; [#uses=1] + store i32 %23, i32* %24, align 4 + ret void + +return: ; preds = %bb7, %entry + ret void +} diff --git a/test/Transforms/GVN/pre-single-pred.ll b/test/Transforms/GVN/pre-single-pred.ll index b735ea9827c..cb71617caed 100644 --- a/test/Transforms/GVN/pre-single-pred.ll +++ b/test/Transforms/GVN/pre-single-pred.ll @@ -1,6 +1,7 @@ ; RUN: llvm-as < %s | opt -gvn -enable-load-pre | llvm-dis | not grep {tmp3 = load} -define i32 @f(i32* nocapture %p, i32 %n) nounwind { +@p = external global i32 +define i32 @f(i32 %n) nounwind { entry: br label %for.cond @@ -13,9 +14,9 @@ for.cond.for.end_crit_edge: ; preds = %for.cond br label %for.end for.body: ; preds = %for.cond - %tmp3 = load i32* %p ; [#uses=1] + %tmp3 = load i32* @p ; [#uses=1] %dec = add i32 %tmp3, -1 ; [#uses=2] - store i32 %dec, i32* %p + store i32 %dec, i32* @p %cmp6 = icmp slt i32 %dec, 0 ; [#uses=1] br i1 %cmp6, label %for.body.for.end_crit_edge, label %for.inc @@ -27,6 +28,6 @@ for.inc: ; preds = %for.body br label %for.cond for.end: ; preds = %for.body.for.end_crit_edge, %for.cond.for.end_crit_edge - %tmp9 = load i32* %p ; [#uses=1] + %tmp9 = load i32* @p ; [#uses=1] ret i32 %tmp9 }