[PATCH] D78486: [SystemZ] Expand vector zero-extend into a shuffle.

Ulrich Weigand via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon May 4 12:22:12 PDT 2020


uweigand added a comment.

In D78486#2014509 <https://reviews.llvm.org/D78486#2014509>, @jonpa wrote:

> > I'm wondering: unless I'm missing something, there's still one specific case where you generate a vperm followed by an unpack (the case where you already had a permute as source).   Wouldn't it be preferable to just use a single vperm there as well?
>
> This is the case where the input is a permute that has no other users, which means that it's being replaced. So instead of vperm + vperm, we get a vperm + unpack, which follows the same reasoning of replacing a vperm with a single unpack.
>
> For example, in the case with three source ops, where A and B first are combined with a permute, and now AB and C are to be combined: instead of permuting AB and C, the AB permute is changed so that AB and C can be unpacked.


Ah, that's what I missed: the case where the first permute has already two different inputs!

Now I'm wondering whether this couldn't also apply in other cases, beyond just perm + unpack.    Would it be possible to handle all cases by recognizing the case where the source of a permute is itself a (single-use) permute early, e.g. in GeneralShuffle::add ?   Hmm, reading that just now, it seems there is already such code:

  else if (Op.getOpcode() == ISD::VECTOR_SHUFFLE && Op.hasOneUse()) {
    // See whether the bytes we need come from a contiguous part of one
    // operand.

So now I'm wondering instead: why does your new code ever trigger in the first place; why isn't this already handled in GeneralShuffle::add?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D78486/new/

https://reviews.llvm.org/D78486





More information about the llvm-commits mailing list