[PATCH] Fix lazy value info computation to improve jump threading for a common case

[Nick Lewycky]

Jiangning Liu wrote:
> Hi,
>
> Attached patch is to fix an issue in lazy value info computation, and
> finally it can improve jump threading for a common case.
>
> Lazy value info computation algorithm tries to use lattice to propagate
> the given value through control flow. The problem of the algorithm is
> the basic block can only be visited once. Once the lattice state is
> changed from "undefined" to other states, the lattice state will be
> cached, and won't be able to be changed any longer.
>
> For the following simple control flow graph like below,
>
> BB1->BB2, BB1->BB3, BB2->BB3, BB2->BB4
>
> When computing a given value on edge BB2->BB3, if B2 doesn't have that
> value info at all, the algorithm will try to ask for information from
> BB2's predecessors, and then return to BB2 again to merge the info from
> predecessors, so BB2 will be visited twice. Unfortunately, at first
> visit of BB2, the lattice state will be changed from "undefined" to
> "overdefined", and then it will not be able to be changed any longer.

That sounds correct. Once overdefined, it's stuck and can't be changed.

How did it get marked overdefined in the first place? Did we 
speculatively mark it overdefined in case we discovered that the 
computation is in terms of ourselves through a phi node or by directly 
depending on ourselves (only possibly if our CFG subgraph is not 
reachable from entry)? Why could we mark it undefined instead? Why 
couldn't we add a new "non-local-lookup-in-progress-here!" state instead?

> This patch is to simply check "overdefined", and make sure the algorithm
> can still move on to propagate the values from predecessor edges to the
> block itself.

That sounds like it could be really bad for compile-time performance.

> The performance experiment for spec shows pretty good result on aarch64
> for this fix, and the 253_perlbmk can even have >5% performance
> improvement.

+  // we need to compute it's value again.

Typo, "it's" -> "its".

+  // check BB2 again, and at this momement BB2 has Overdefined value 
for %v in

Typo, "momement" -> "moment".

+  // BB2. So we should have to follow data flow propagation algirithm 
to get the

Typo, "algirithm" -> "algorithm".

BTW, there are some unreviewed patches for lazy value info here: 
http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20140127/203418.html 
. The problem description sounds like it could be related, it's not 
obviously not the same problem to me. Did his patch come with tests? 
Does your patch fix those tests too?

Nick