E-SSA and Predicate Info (was RE: [PATCH] D28459: Make processing @llvm.assume more efficient - Add affected values to the assumption cache)

[Hal Finkel via llvm-commits]

On 01/14/2017 07:19 PM, Daniel Berlin wrote:
>
>
> On Sat, Jan 14, 2017 at 4:47 PM, Hal Finkel <hfinkel at anl.gov 
> <mailto:hfinkel at anl.gov>> wrote:
>
>
>     On 01/14/2017 05:21 PM, Daniel Berlin wrote:
>>
>>     In any case, if you want to play with it, it's here:
>>     https://github.com/dberlin/llvm-gvn-rewrite/tree/newgvn-predicateinfo
>>     <https://github.com/dberlin/llvm-gvn-rewrite/tree/newgvn-predicateinfo>
>>
>>     -print-predicateinfo -analyze will give you info.
>>
>>     -newgvn will process simple equality and inequality right now
>>     using that info[1]
>>
>>     This  is pretty much as cheap as you can make it.
>>     We compute it in O(number of uses of comparison operations that
>>     are used in terminators) worst case time.
>>     So it's not even O(number of instructions) unless your program is
>>     only comparisons and branches :P
>>
>>     This includes pruning - it will not insert predicate info copies
>>     except where they are actually used on a branch.
>>
>>     (the same O(uses) algorithm works for general SSA renaming as well)
>>
>>     Adding assume support would just require coming up with a copy
>>     operation, and doing local numbering in the assume blocks only
>>     (so we get def vs use order right in that block).
>
>     Looks good, thanks! The code you have in
>     PredicateInfo::buildPredicateInfo looks essentially like the code
>     I have in AssumptionCache::updateAffectedValues; we just need to
>     update the PredicateInfo version to catch a few more cases that
>     the assumptions need.
>
>
> I'm working on it.
>
>
>     I don't understand what you mean by "in the assume blocks only."
>     I'd think we'd need to treat these just like dominating
>     conditional-branch conditions, so we'd need to rename all uses
>     dominated by the assumption in all blocks.
>
>
> Yes, i meant The local ordering only has to be done in the assume 
> block only, because global ordering is taken care of by dominator tree.
> The way it works is by sorting in DFS order (to avoid walking the 
> entire dominator tree just for ordering effect).
> Globally, we have DFS of dominator tree to order things.
> Inside the same block, you need a local DFS order (order of appearance)
> The only blocks where you need such an ordering is blocks with 
> assumes, because we don't know where in the block the assume is.
> For conditionals, the value gets generated on the true/false edge, so 
> it always comes first in the block, and we don't need any real 
> single-block ordering for it.
>
> WIthout a local ordering for assume, we will try to rename
>
> bb1:
>
> foo = 5
> use foo
> result = icmp eq foo, bar
> assume(result)
>
> into
>
> foo = 5
> use foonew
> result = icmp eq foo, bar
> assume(result)
> foonew = predicateinfo(foo)
>
> because we won't realize the def comes after the use.

Makes sense. Thanks for explaining.

>
>
> Though now that i re-read the lang-ref, it says:
> "The intrinsic allows the optimizer to assume that the provided 
> condition is always true whenever the control flow reaches the 
> intrinsic call."
>
> Does this mean i can make go upwards  as long as the assume 
> post-dominates the  use?

Yes. The current code even checks for this, at least in a restricted 
sense, at the end of llvm::isValidAssumeForContext. For assumes and 
context instructions in the same block, if the assume post-dominates but 
everything in between is safe to speculate, then we'll still find it. 
Speculation safety is a stronger condition than necessary (because 
faults that are UB get to travel backwards too, so we don't need to 
guard against them), but does serve to limit the cost of the search 
while still allowing innocent code motion not to affect our ability to 
apply assumptions. If we had a more-efficient way to check dominance, we 
might be more forgiving, although we still need to check that there are 
no potentially-throwing calls in between.

>
> If so, the above would be valid as long as i change it to:
>
> foo = 5
> foonew = predicateinfo(foo)
> use foonew
> result = icmp eq foo, bar
> assume(result)
>
> which we could do easily.

Sounds good to me. The foonew does not need to be dominated by the 
result because that's a side table?

  -Hal

>
>

-- 
Hal Finkel
Lead, Compiler Technology and Programming Languages
Leadership Computing Facility
Argonne National Laboratory

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170114/d527e033/attachment.html>