<div style="font-family:arial,helvetica,sans-serif;font-size:10pt">Thanks for writing up a nice testcase! Yes, this patch looks good, please commit it.<div><br></div><div><div><div class="gmail_quote">On 10 December 2012 13:54, Arnold Schwaighofer <span dir="ltr"><<a href="mailto:arnold.schwaighofer@gmail.com" target="_blank">arnold.schwaighofer@gmail.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Hi Nick,<br>
<br>
thanks for looking at this, the more eyes the better (also thanks to<br>
Dan and Duncan!)<br>
<br>
No, for this case it will return MayAlias because it will ask<br>
<br>
aliasCheck(%y, %pn1)<br>
<br>
which will in turn call (because of aliasCheck(%y, phi(%x,%y)))<br>
<br>
aliasCheck(%y, %x) and aliasCheck(%y, %y) => alias<br>
<br>
<br>
<br>
cat phi-speculation.ll<br>
<br>
arget datalayout =<br>
"e-p:64:64:64-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:64:64-f32:32:32-f64:64:64-v64:64:64-v128:128:128-a0:0:64-s0:64:64-f80:128:128-n8:16:32:64-S128"<br>
<br>
; RUN: opt < %s -basicaa -aa-eval -print-all-alias-modref-info<br>
-disable-output 2>&1 | FileCheck %s<br>
<br>
<br>
<br>
define i32 @test_noalias_3(i8* noalias %x, i8* noalias %y, i8* noalias<br>
%z, i32 %count) {<br>
entry:<br>
br label %while.body<br>
<br>
while.body:<br>
%num = phi i32 [ %count, %entry ], [ %dec, %while.body ]<br>
%ptr_phi = phi i8* [ %x, %entry ], [ %z, %while.body ]<br>
%ptr2_phi = phi i8* [ %y, %entry ], [ %ptr_phi, %while.body ]<br>
%dec = add nsw i32 %num, -1<br>
%tobool = icmp eq i32 %dec, 0<br>
br i1 %tobool, label %the_exit, label %while.body<br>
<br>
the_exit:<br>
ret i32 1<br>
}<br>
<br>
build-patched/Release+Asserts/bin/opt -basicaa --aa-eval<br>
-print-all-alias-modref-info -disable-output < phi-speculation.ll<br>
Function: test_noalias_3: 5 pointers, 0 call sites<br>
NoAlias: i8* %x, i8* %y<br>
NoAlias: i8* %x, i8* %z<br>
NoAlias: i8* %y, i8* %z<br>
MayAlias: i8* %ptr_phi, i8* %x<br>
NoAlias: i8* %ptr_phi, i8* %y<br>
MayAlias: i8* %ptr_phi, i8* %z<br>
MayAlias: i8* %ptr2_phi, i8* %x<br>
MayAlias: i8* %ptr2_phi, i8* %y<br>
MayAlias: i8* %ptr2_phi, i8* %z<br>
MayAlias: i8* %ptr2_phi, i8* %ptr_phi<br>
===== Alias Analysis Evaluator Report =====<br>
10 Total Alias Queries Performed<br>
4 no alias responses (40.0%)<br>
6 may alias responses (60.0%)<br>
0 partial alias responses (0.0%)<br>
0 must alias responses (0.0%)<br>
Alias Analysis Evaluator Pointer Alias Summary: 40%/60%/0%/0%<br>
Alias Analysis Mod/Ref Evaluator Summary: no mod/ref!<br>
<div class="HOEnZb"><div class="h5"><br>
On Mon, Dec 10, 2012 at 3:07 PM, Nick Lewycky <<a href="mailto:nlewycky@google.com">nlewycky@google.com</a>> wrote:<br>
> On 8 December 2012 10:12, Hal Finkel <<a href="mailto:hfinkel@anl.gov">hfinkel@anl.gov</a>> wrote:<br>
>><br>
>> Nick,<br>
>><br>
>> You had okayed the previous version of this. Would you like to look at<br>
>> this one as well?<br>
><br>
><br>
> Thanks for the ping!<br>
><br>
> I took a look, and I must be missing something. Won't this miscompile:<br>
><br>
> bb2:<br>
> %pn1 = phi i8* [ %x, %bb1 ], [ %y, %bb2 ]<br>
> %pn2 = phi i8* [ %z, %bb1 ], [ %pn1, %bb2 ]<br>
><br>
> when %x, %y and %z are all NoAlias? It will speculate that %pn1 and %pn2<br>
> don't alias and the proceed to confirm that just because it speculated it.<br>
><br>
> Nick<br>
><br>
>><br>
>> -Hal<br>
>><br>
>> ----- Original Message -----<br>
>> > From: "Dan Gohman" <<a href="mailto:dan433584@gmail.com">dan433584@gmail.com</a>><br>
>> > To: "Hal Finkel" <<a href="mailto:hfinkel@anl.gov">hfinkel@anl.gov</a>><br>
>> > Cc: "Arnold Schwaighofer" <<a href="mailto:arnold.schwaighofer@gmail.com">arnold.schwaighofer@gmail.com</a>>, "Commit<br>
>> > Messages and Patches for LLVM"<br>
>> > <<a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a>><br>
>> > Sent: Thursday, December 6, 2012 4:32:32 PM<br>
>> > Subject: Re: [llvm-commits] [PATCH] Optimistically analyse phi cycles in<br>
>> > BasicAA<br>
>> ><br>
>> > The basic idea sounds fine.<br>
>> ><br>
>> > Dan<br>
>> ><br>
>> > On Thu, Dec 6, 2012 at 9:31 AM, Hal Finkel <<a href="mailto:hfinkel@anl.gov">hfinkel@anl.gov</a>> wrote:<br>
>> > > Dan,<br>
>> > ><br>
>> > > Do you have an opinion on this?<br>
>> > ><br>
>> > > Thanks again,<br>
>> > > Hal<br>
>> > ><br>
>> > > ----- Original Message -----<br>
>> > >> From: "Arnold Schwaighofer" <<a href="mailto:arnold.schwaighofer@gmail.com">arnold.schwaighofer@gmail.com</a>><br>
>> > >> Cc: "Commit Messages and Patches for LLVM"<br>
>> > >> <<a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a>><br>
>> > >> Sent: Wednesday, November 21, 2012 9:24:13 AM<br>
>> > >> Subject: Re: [llvm-commits] [PATCH] Optimistically analyse phi<br>
>> > >> cycles in BasicAA<br>
>> > >><br>
>> > >> Also send to list ...<br>
>> > >><br>
>> > >> On Wed, Nov 21, 2012 at 3:37 AM, Duncan Sands <<a href="mailto:baldrick@free.fr">baldrick@free.fr</a>><br>
>> > >> wrote:<br>
>> > >> ><br>
>> > >> > what happens if the recursion bails out due to hitting a<br>
>> > >> > recursion<br>
>> > >> > limit?<br>
>> > >> > Will the conservatively correct MayAlias be returned in that<br>
>> > >> > case?<br>
>> > >><br>
>> > >> Yes, because when we hit the limit in the recursion we return<br>
>> > >> MayAlias<br>
>> > >> from this recursion. This result will be merged with the phi's<br>
>> > >> alias<br>
>> > >> result, setting the phi to MayAlias.<br>
>> > >><br>
>> > >> AliasResult Alias = NoAlias;<br>
>> > >> AliasResult OrigAliasResult = AliasCache[Locs]; // Is<br>
>> > >> MayAlias<br>
>> > >> if we have never seen this node.<br>
>> > >> AliasCache[Locs] = NoAlias;<br>
>> > >><br>
>> > >> for (unsigned i = 0, e = PN->getNumIncomingValues(); i != e;<br>
>> > >> ++i) {<br>
>> > >> AliasResult ThisAlias =<br>
>> > >> [1]<br>
>> > >> aliasCheck(PN->getIncomingValue(i), PNSize, PNTBAAInfo,<br>
>> > >><br>
>> > >> PN2->getIncomingValueForBlock(PN->getIncomingBlock(i)),<br>
>> > >> V2Size, V2TBAAInfo);<br>
>> > >> Alias = MergeAliasResults(ThisAlias, Alias);<br>
>> > >> if (Alias == MayAlias)<br>
>> > >> break;<br>
>> > >> }<br>
>> > >><br>
>> > >> // Reset if speculation failed.<br>
>> > >> if (Alias != NoAlias)<br>
>> > >> AliasCache[Locs] = OrigAliasResult;<br>
>> > >><br>
>> > >> We either process all nodes in the phis' input graph and<br>
>> > >> eventually<br>
>> > >> see a MayAlias result or we give up early and return MayAlias<br>
>> > >> through<br>
>> > >> the recursion.<br>
>> > >> The interesting case when we handle two phi nodes. We assume the<br>
>> > >> phis<br>
>> > >> to be NoAlias and they are not. Let say we process all inputs<br>
>> > >> recursively then we eventually will see an input that is MayAlias,<br>
>> > >> this will be propagate up to the call aliasCheck [1] above,<br>
>> > >> resulting<br>
>> > >> in the phi to be set to MayAlias.<br>
>> > >> In the case were we hit the recursion limit this again will happen<br>
>> > >> in<br>
>> > >> the call to aliasCheck [1] which then returns MayAlias and again<br>
>> > >> the<br>
>> > >> phis' result will be reset to MayAlias.<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>
>> > >><br>
>> > ><br>
>> > > --<br>
>> > > Hal Finkel<br>
>> > > Postdoctoral Appointee<br>
>> > > Leadership Computing Facility<br>
>> > > Argonne National Laboratory<br>
>> ><br>
>><br>
>> --<br>
>> Hal Finkel<br>
>> Postdoctoral Appointee<br>
>> Leadership Computing Facility<br>
>> Argonne National Laboratory<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>
><br>
><br>
</div></div></blockquote></div><br></div></div></div>