[llvm] r198290 - BasicAA: Fix value equality and phi cycles

[Nick Lewycky]

On 01/01/2014 07:31 PM, Arnold Schwaighofer wrote:
> Author: arnolds
> Date: Wed Jan  1 21:31:36 2014
> New Revision: 198290
>

I don't entirely understand why the fix isn't to speculate less often in 
aliasPHI?

>     // Are we checking for alias of the same value?
> -  if (V1 == V2) return MustAlias;
> +  if (isValueEqual(V1, V2)) return MustAlias;

This demands a massive explanation of what on earth is going on. Of 
course %x mustalias %x! (Yes, I've read the PR and understand it what's 
going on, but I'd much rather see code that explicitly dealt with the 
"we're in a loop and speculating on PHIs case" broken out.)

> +bool BasicAliasAnalysis::isValueEqual(const Value *V, const Value *V2) {
> +  if (V != V2)
> +    return false;
> +
> +  const Instruction *Inst = dyn_cast<Instruction>(V);
> +  if (!Inst)
> +    return true;
> +
> +  // Use the dominance if available.
> +  DT = getAnalysisIfAvailable<DominatorTree>();
> +  if (DT) {
> +    if (VisitedPhiBBs.size() > MaxNumPhiBBsValueDominanceCheck)
> +      return false;
> +
> +    // Make sure that the visited phis are dominated by the Value.
> +    for (SmallPtrSet<const BasicBlock *, 8>::iterator
> +             PI = VisitedPhiBBs.begin(),
> +             PE = VisitedPhiBBs.end();
> +         PI != PE; ++PI)
> +      if (!DT->dominates(Inst, *PI))
> +        return false;

I haven't thought really hard about this, but did you want to use 
llvm::isPotentiallyReachable?

> Modified: llvm/trunk/test/Analysis/BasicAA/2006-03-03-BadArraySubscript.ll
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Analysis/BasicAA/2006-03-03-BadArraySubscript.ll?rev=198290&r1=198289&r2=198290&view=diff
> ==============================================================================
> --- llvm/trunk/test/Analysis/BasicAA/2006-03-03-BadArraySubscript.ll (original)
> +++ llvm/trunk/test/Analysis/BasicAA/2006-03-03-BadArraySubscript.ll Wed Jan  1 21:31:36 2014
> @@ -1,4 +1,4 @@
> -; RUN: opt < %s -basicaa -aa-eval -disable-output 2>&1 | FileCheck %s
> +; RUN: opt < %s -domtree -basicaa -aa-eval -disable-output 2>&1 | FileCheck %s

Adding -domtree to all these tests makes me very sad.

Nick