[PATCH] D38388: [DAGCombiner, x86] convert insertelement of bitcasted vector into shuffle

Sanjay Patel via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Sun Oct 1 10:19:01 PDT 2017


spatel added a comment.

In https://reviews.llvm.org/D38388#885413, @RKSimon wrote:

> Has the legalization requirements affected whether we can abandon https://reviews.llvm.org/D38316?


It's a gray area (to me at least). This patch is enough to solve the motivating bug:
https://bugs.llvm.org/show_bug.cgi?id=34716 
...and it captures the more general case of inserting into a non-undef vector too, but you're right - it no longer handles the 512-bit vector example with a potentially pre-AVX512 target that Eli may have been suggesting in the other review.

So the considerations are (and I don't have a strong opinion either way):

1. The IR canonicalization of https://reviews.llvm.org/D38316 can enable an IR improvement (less instructions in the motivating bug), so it still has some value. Is that value enough to justify the cost?
2. Do we want to add a different target-independent IR-level fold to kill the extra insertelement instruction for a non-undef insertion? That one seems more complicated, so we'd at least want to know how that pattern was created.


https://reviews.llvm.org/D38388





More information about the llvm-commits mailing list