[PATCH] D19338: New code hoisting pass based on GVN (optimistic approach)

[Daniel Berlin via llvm-commits]

To be specific (and feel free to tell me why i'm wrong, these functions are
a bit hard to decipher without comments):

For starters, for loads/stores I don't understand why you use
gatherallpaths and then check the memoryuses, instead of calculating
Nearest common dominator (or postdominator, depending)(hoist point, blocks
for all the uses).


Let's ignore the may-throw/etc issues for a second (which can be done with
a single block test)

For scalar computations, hoist point safety depends on whether you can
recompute the operands at that dominator, nothing else.

For load/stores, it's easier. For loads, if the memory state (ie the thing
the clobbering memorydef defines) dominates your hoistpoint, then it
becomes the same scalar question, because if you can re-make the load, you
know the memory-state will be the same.
For stores, if you are sinking, you are defining the memory state in a
given block, the only question is whether that memory state is killed
before it the use.

Checking domination is not necessary, the only way it can be true is if
every store produces the same VN, and it's intersection of all successors
to the sink point.

If you are trying to *hoist* stores, it's a much simpler problem, the point
you can hoist to is
nearest-common-dominator(getClobberingDefinition(store)->block, blocks of
all uses).



On Mon, Jun 6, 2016 at 8:43 AM, Daniel Berlin <dberlin at dberlin.org> wrote:

> dberlin added inline comments.
>
> ================
> Comment at: llvm/lib/Transforms/Scalar/GVNHoist.cpp:278
> @@ +277,3 @@
> +  // Return true when there are users of A in one of the BBs of Paths.
> +  bool hasMemoryUseOnPaths(MemoryAccess *A,
> +                           SmallPtrSetImpl<const BasicBlock *> &Paths) {
> ----------------
> I feel like this + gatherallblocks is really an up-safe/down-safe
> computation (depending on whether it's a load or store), and can be done
> much saner than you are doing it.
>
> In particular, this seems a really complicated and expensive way to
> compute this property, compared to how most PRE papers do it.
>
>
> http://reviews.llvm.org/D19338
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160606/11c01be6/attachment.html>