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

[Ulrich Weigand via Phabricator via llvm-commits]

uweigand added a comment.

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

> > 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?
>
> I think that be two different cases, where my handling is addressing a *new* permute resulting from combining two operands, when trying to combine (unpack) a third source operand with that first (new) permute.


Ah, I see.  So this permute is one that had just been generated via getGeneralPermuteNode earlier in the loop?   It's a bit unfortunate to first create that ISD node and then right away delete it again.

Maybe one option to try would be to do the check whether this permutation looks like an unpack (possibly requiring a permutation) at the top level *before* the loop.   If this is true, then apply the permutation (in reverse) to the Bytes array, remove the zerovec from the Ops list, and run through the loop.   At the end, remember to actually generate the unpack node.   This would also remove the need to special-case handing the zerovec through the loop.

The whole operation would be worthwhile only if by performing the unpack we can remove the zerovec (i.e. it isn't still used elsewhere), and if by removing the zerovec the depth of the resulting tree (i.e. the critical path length) is also reduced.   The depth is the log2 of the number of operands (rounded up), so if reducing the zerovec takes us from 3 to 2 ops, it is worthwhile, but if it takes us from 2 to 1 ops (or from 4 to 3 ops), then it probably is not, since adding the unpack would then just add one more instruction to the critical path.

Does this make sense?


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

https://reviews.llvm.org/D78486