[PATCH] D26224: NewGVN

Daniel Berlin via llvm-commits llvm-commits at lists.llvm.org
Tue Dec 13 19:49:20 PST 2016


On Tue, Dec 13, 2016 at 7:27 PM, Hal Finkel via Phabricator <
reviews at reviews.llvm.org> wrote:

> hfinkel added a comment.
>
> A few more comments on the comments, otherwise, I'm fine with committing
> this and working on it in-tree. Thanks for your work on this!
>
>
>
> ================
> Comment at: include/llvm/Transforms/Scalar/GVNExpression.h:66
> +    // Compare the expression type for anything but load and store.
> +    // This is needed for load coercion.
> +    if (getExpressionType() != ET_Load &&
> ----------------
> What does load coercion mean in this context? Also, for loads and stores
> we set the opcode to 0 and that should be noted somewhere here.
>

We duplicate the functionality of current GVN (though not in this patch),
which "coerces" parts of existing loads to get values out of them.

So for example,
given

load i32, %ptr
and load i16, %ptr
and
load i16, %ptr + 1 (or whatever the second half is)

Current GVN understands it can pull the second two values out of the first
one.

It also understands, given

load i16
load i16
load i32


that it can widen the first load to i32 and eliminate the second two.

In practice, the types of loads/stores don't matter anyway (ignoring
non-integral pointer types, which we'd probably just pass on), we can
always make them correct.

>
>
> ================
> Comment at: lib/Transforms/Scalar/NewGVN.cpp:648
> +                                                   const BasicBlock *B) {
> +  // FIXME: Add bundle operators for calls.
> +  CallExpression *E =
> ----------------
> bundle operators -> operand bundles
>
>
> https://reviews.llvm.org/D26224
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20161213/47502dd7/attachment.html>


More information about the llvm-commits mailing list