[llvm] r215343 - In LVI(Lazy Value Info), originally value on a BB can only be caculated once,

[Hans Wennborg]

On Thu, Nov 20, 2014 at 11:27 AM, Pete Cooper <peter_cooper at apple.com> wrote:
> I came across this independently a few days ago.  My example was similar.  I
> had
>
> if (x == 0) {
>   if (…) {  … } else { … }
>   ++x
> }

I'm still not very familiar with this analysis and its interaction
with jump threading. Could you turn your example into a compilable
function that I can experiment with?


> The problem here is that as we walk from ++x up, we visit the BBs on the
> inner if statement.  Their predecessors haven’t been processed yet so we
> return out from solveBlockValueNonLocal which leaves them in the
> ‘overdefined’ state.  We eventually get to ‘x == 0’ and start the walk back
> down the CFG.  But on the walk back down we do the following check and
> immediately bail.  So intermediate BBs get set to overdefined because of
> unvisited predecessors and never leave that state.
>
> if (!BBLV.isUndefined()) {
>     DEBUG(dbgs() << "  reuse BB '" << BB->getName() << "' val=" << BBLV
> <<'\n');
>
>
>
>     // Since we're reusing a cached value here, we don't need to update the
>     // OverDefinedCahce.  The cache will have been properly updated
>     // whenever the cached value was inserted.
>     ODCacheUpdater.markResult(false);
>     return true;
>   }
>
> So, here’s the patch I have to fix the problem.  I believe it has no
> algorithmic complexity issues as we do the same walk up the CFG from before,
> but as we walk back down we will retry calling solveBlockValueNonLocal now
> that all predecessors have a state.  I don’t have an IR test case this
> minute, but I can get one tonight if needed.
>
> @@ -672,8 +672,16 @@ bool
> LazyValueInfoCache::solveBlockValueNonLocal(LVILatticeVal &BBLV,
>        return true;
>      }
>    }
> -  if (EdgesMissing)
> +  if (EdgesMissing) {
> +    // If we haven't yet processed all the predecessors, then put this
> +    // value back to undefined so that we'll revisit it again.  Otherwise
> it
> +    // will be 'overdefined' once we return from here and we'll think that
> +    // this is its final state, while in fact we don't know the state of
> this
> +    // value in this BB yet.
> +    LVILatticeVal Result;
> +    BBLV = Result;
>      return false;
> +  }

I applied your patch, but it seems to be causing miscompiles, as one
of the programs got into an infinite loop and consumed all my memory.

 - Hans