[PATCH] D21041: [GVN] PRE can't hoist loads across calls in general.

[Daniel Berlin via llvm-commits]

>
>
> I'm hesitant to add a new analysis pass because keeping the analysis
> up-to-date is inevitably painful... especially given that GVN probably
> wants to check this at higher resolution than just per-block.


Again, it's very wasteful for it to have to know/compute (even once), where
the blocks that it has to look at in more detail are, when nobody makes
things *more* maythrow then they were before.

If the CFG properly represented "this block, somewhere, throws", it would
know "okay, if i want to hoist out of this block, i have to look harder".

Instead, we now have a bunch of passes that are, at a minimum, going to
look at every instruction in each block and start keeping track of the same
info. In fact, every patch we've done so far here (even mine!) tries to
recompute this very info so it can figure out where it needs to look harder
in constant time.

Also, the current PRE does not really do good PRE, and makes no attempt to
place instructions like a real PRE would.
Any real PRE or store sinking pass is going to want to know what blocks are
"transparent", that is, it is possible to hoist or sink through.

MemorySSA will tell them if the memory state is killed in the block in
constant time.
SSA will tell them if the operands are killed or reusable in the block in
constant time.

The per-block bit you are talking about is precisely the last piece that
tells you if a block is transparent in constant time.

Without it, you must touch every instruction in the program to know which
blocks are transparent.



> Maybe it's the right approach, though.
>
> > At minimum, the patch should be rewritten as an extension to
> isValueFullyAvailableInBlock; that's the bit that's supposed to be
> reasoning about anticipation for LoadPRE.
>
>
> I'm not following; this isn't something we need to check on a
> per-predecessor basis.  The issue is basically whether it's legal to hoist
> a given load from the middle of its parent BB to the beginning of that BB.
>

This is just because of how current PRE works (which, as mentioned, is
pretty wasteful). If you only want to fix that behavior, then yeah, compute
it once, use that.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160614/4926948c/attachment.html>