<div dir="ltr">Thanks! Committed r207853.</div><div class="gmail_extra"><br><br><div class="gmail_quote">On Thu, May 1, 2014 at 4:08 PM, Justin Bogner <span dir="ltr"><<a href="mailto:mail@justinbogner.com" target="_blank">mail@justinbogner.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">This is obviously correct. LGTM.<br>
<div><div class="h5"><br>
Akira Hatanaka <<a href="mailto:ahatanak@gmail.com">ahatanak@gmail.com</a>> writes:<br>
> This patch fixes GVN::AnalyzeLoadAvailability to pass the phi-translated<br>
> pointer address of the load instruction that is processed instead of the<br>
> untranslated address to AnalyzeLoadFromClobberingLoad.<br>
><br>
> Without the fix, GVN::AnalyzeLoadAvailability incorrectly determines a load<br>
> value is available or unavailable in a basic block, which causes load-PRE to<br>
> incorrectly determine a load is partially redundant when it isn't (see first<br>
> function in test case) or it isn't partially redundant when it is (second<br>
> function in test case).<br>
><br>
> I tested this patch running the test suite and didn't see any failing tests.<br>
><br>
> <rdar://problem/16638765>.<br>
><br>
> Please review.<br>
><br>
><br>
</div></div>> diff --git a/lib/Transforms/Scalar/GVN.cpp b/lib/Transforms/Scalar/GVN.cpp<br>
> index e7156fd..0253a05 100644<br>
> --- a/lib/Transforms/Scalar/GVN.cpp<br>
> +++ b/lib/Transforms/Scalar/GVN.cpp<br>
> @@ -1421,8 +1421,7 @@ void GVN::AnalyzeLoadAvailability(LoadInst *LI, LoadDepVect &Deps,<br>
> // If this is a clobber and L is the first instruction in its block, then<br>
> // we have the first instruction in the entry block.<br>
> if (DepLI != LI && Address && DL) {<br>
> - int Offset = AnalyzeLoadFromClobberingLoad(LI->getType(),<br>
> - LI->getPointerOperand(),<br>
> + int Offset = AnalyzeLoadFromClobberingLoad(LI->getType(), Address,<br>
> DepLI, *DL);<br>
><br>
> if (Offset != -1) {<br>
> diff --git a/test/Transforms/GVN/load-pre-nonlocal.ll b/test/Transforms/GVN/load-pre-nonlocal.ll<br>
> new file mode 100644<br>
> index 0000000..7bac1b7<br>
> --- /dev/null<br>
> +++ b/test/Transforms/GVN/load-pre-nonlocal.ll<br>
> @@ -0,0 +1,87 @@<br>
> +; RUN: opt -S -o - -basicaa -domtree -gvn %s | FileCheck %s<br>
> +<br>
> +target datalayout = "e-m:o-i64:64-f80:128-n8:16:32:64-S128"<br>
> +<br>
> +%struct.S1 = type { i32, i32 }<br>
> +<br>
> +@a2 = common global i32* null, align 8<br>
> +@a = common global i32* null, align 8<br>
> +@s1 = common global %struct.S1 zeroinitializer, align 8<br>
> +<br>
> +; Check that GVN doesn't determine %2 is partially redundant.<br>
> +<br>
> +; CHECK-LABEL: define i32 @volatile_load<br>
> +; CHECK: for.body:<br>
> +; CHECK: %2 = load i32*<br>
> +; CHECK: %3 = load volatile i32*<br>
> +; CHECK: for.cond.for.end_crit_edge:<br>
> +<br>
> +define i32 @volatile_load(i32 %n) {<br>
> +entry:<br>
> + %cmp6 = icmp sgt i32 %n, 0<br>
> + br i1 %cmp6, label %<a href="http://for.body.lr.ph" target="_blank">for.body.lr.ph</a>, label %for.end<br>
> +<br>
> +<a href="http://for.body.lr.ph" target="_blank">for.body.lr.ph</a>:<br>
> + %0 = load i32** @a2, align 8, !tbaa !1<br>
> + %1 = load i32** @a, align 8, !tbaa !1<br>
> + br label %for.body<br>
> +<br>
> +for.body:<br>
> + %indvars.iv = phi i64 [ 0, %<a href="http://for.body.lr.ph" target="_blank">for.body.lr.ph</a> ], [ %indvars.iv.next, %for.body ]<br>
> + %s.09 = phi i32 [ 0, %<a href="http://for.body.lr.ph" target="_blank">for.body.lr.ph</a> ], [ %add, %for.body ]<br>
> + %p.08 = phi i32* [ %0, %<a href="http://for.body.lr.ph" target="_blank">for.body.lr.ph</a> ], [ %incdec.ptr, %for.body ]<br>
> + %2 = load i32* %p.08, align 4, !tbaa !5<br>
> + %arrayidx = getelementptr inbounds i32* %1, i64 %indvars.iv<br>
> + store i32 %2, i32* %arrayidx, align 4, !tbaa !5<br>
> + %3 = load volatile i32* %p.08, align 4, !tbaa !5<br>
> + %add = add nsw i32 %3, %s.09<br>
> + %indvars.iv.next = add nuw nsw i64 %indvars.iv, 1<br>
> + %incdec.ptr = getelementptr inbounds i32* %p.08, i64 1<br>
> + %lftr.wideiv = trunc i64 %indvars.iv.next to i32<br>
> + %exitcond = icmp ne i32 %lftr.wideiv, %n<br>
> + br i1 %exitcond, label %for.body, label %for.cond.for.end_crit_edge<br>
> +<br>
> +for.cond.for.end_crit_edge:<br>
> + %add.lcssa = phi i32 [ %add, %for.body ]<br>
> + br label %for.end<br>
> +<br>
> +for.end:<br>
> + %s.0.lcssa = phi i32 [ %add.lcssa, %for.cond.for.end_crit_edge ], [ 0, %entry ]<br>
> + ret i32 %s.0.lcssa<br>
> +}<br>
> +<br>
> +; %1 is partially redundant if %0 can be widened to a 64-bit load.<br>
> +<br>
> +; CHECK-LABEL: define i32 @overaligned_load<br>
> +; CHECK: if.end:<br>
> +; CHECK-NOT: %1 = load i32*<br>
> +<br>
> +define i32 @overaligned_load(i32 %a, i32* nocapture %b) {<br>
> +entry:<br>
> + %cmp = icmp sgt i32 %a, 0<br>
> + br i1 %cmp, label %if.then, label %if.else<br>
> +<br>
> +if.then:<br>
> + %0 = load i32* getelementptr inbounds (%struct.S1* @s1, i64 0, i32 0), align 8, !tbaa !5<br>
> + br label %if.end<br>
> +<br>
> +if.else:<br>
> + %arrayidx = getelementptr inbounds i32* %b, i64 2<br>
> + store i32 10, i32* %arrayidx, align 4, !tbaa !5<br>
> + br label %if.end<br>
> +<br>
> +if.end:<br>
> + %i.0 = phi i32 [ %0, %if.then ], [ 0, %if.else ]<br>
> + %p.0 = phi i32* [ getelementptr inbounds (%struct.S1* @s1, i64 0, i32 0), %if.then ], [ %b, %if.else ]<br>
> + %add.ptr = getelementptr inbounds i32* %p.0, i64 1<br>
> + %1 = load i32* %add.ptr, align 4, !tbaa !5<br>
> + %add1 = add nsw i32 %1, %i.0<br>
> + ret i32 %add1<br>
> +}<br>
> +<br>
> +!1 = metadata !{metadata !2, metadata !2, i64 0}<br>
> +!2 = metadata !{metadata !"any pointer", metadata !3, i64 0}<br>
> +!3 = metadata !{metadata !"omnipotent char", metadata !4, i64 0}<br>
> +!4 = metadata !{metadata !"Simple C/C++ TBAA"}<br>
> +!5 = metadata !{metadata !6, metadata !6, i64 0}<br>
> +!6 = metadata !{metadata !"int", metadata !3, i64 0}<br>
> _______________________________________________<br>
> llvm-commits mailing list<br>
> <a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br>
> <a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br>
</blockquote></div><br></div>