[llvm-commits] [PATCH] Phi speculation improvement for BasicAA

[Hal Finkel]

----- Original Message -----
> From: "Eli Friedman" <eli.friedman at gmail.com>
> To: "Hal Finkel" <hfinkel at anl.gov>
> Cc: "Commit Messages and Patches for LLVM" <llvm-commits at cs.uiuc.edu>
> Sent: Friday, November 16, 2012 6:30:49 PM
> Subject: Re: [llvm-commits] [PATCH] Phi speculation improvement for BasicAA
> 
> On Fri, Nov 16, 2012 at 4:16 PM, Hal Finkel <hfinkel at anl.gov> wrote:
> > Hello,
> >
> > As a followup, and partial solution, to PR14351, the attached patch
> > removes some of the special significance of the first incoming phi
> > value in the phi aliasing checking logic in BasicAA. In the
> > context of a loop, the current logic assumes that the first
> > incoming value is the interesting one (meaning that it is the one
> > that comes from outside the loop), but this is often not the case.
> > With this patch, we now test first the incoming value that comes
> > from a block other than the parent of the phi being tested. This
> > is important for, among other things, alias-analysis-based
> > instruction dependency breaking for instruction scheduling.
> >
> > As noted in the PR, Arnold is currently working on a more general
> > solution. In the mean time, this fix captures several cases I need
> > (and has an associated test case). Please review.
> 
> --- a/lib/Analysis/BasicAliasAnalysis.cpp
> +++ b/lib/Analysis/BasicAliasAnalysis.cpp
> @@ -1065,9 +1065,15 @@ BasicAliasAnalysis::aliasPHI(const PHINode
> *PN,
> uint64_t PNSize,
>        if (PN > V2)
>          std::swap(Locs.first, Locs.second);
> 
> +      // Find the first incoming phi value not from its parent.
> +      unsigned f = 0;
> +      for (; PN->getIncomingBlock(f) == PN->getParent(); ++f);
> 
> Try to avoid for loops with an empty body, e.g. "while
> (PN->getIncomingBlock(f) == PN->getParent()) { ++f; }".  Or if you
> really like the for loop, at least put the ";" on a separate line.
> 
> Also, this loop isn't guaranteed to terminate in unreachable code.
> 
> 
> It would be nice if you could simplify the testcase a bit more.

Eli, thanks! I've turned to loop into a while (and fixed it so that it won't run off the end of the list in unreachable code), and cleanup up the test case. How's this?

 -Hal

> 
> -Eli
> 

-- 
Hal Finkel
Postdoctoral Appointee
Leadership Computing Facility
Argonne National Laboratory
-------------- next part --------------
A non-text attachment was scrubbed...
Name: phi_spec_order2.patch
Type: text/x-patch
Size: 5683 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20121116/f6b18872/attachment.bin>