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

[Eli Friedman]

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