[PATCH] D120018: [AArch64] Alter mull shuffle(ext(..)) combine to work on buildvectors

[Nicholas Guy via Phabricator via llvm-commits]

NickGuy added a comment.

Looks good overall, with potential wins in both performance and codesize. Just 1 minor thing (and a nitpick about a comment).



================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:13512
-/// Combines a dup(sext/zext) node pattern into sext/zext(dup)
-/// making use of the vector SExt/ZExt rather than the scalar SExt/ZExt
-static SDValue performCommonVectorExtendCombine(SDValue VectorShuffle,
----------------
Do we want to keep this comment (or replace it, rather than remove it)?


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:13542-13543
+  EVT PreExtendVT = VT.changeVectorElementType(PreExtendType);
+  EVT PreExtendLegalType =
+      PreExtendType.getScalarSizeInBits() < 32 ? MVT::i32 : PreExtendType;
+  SDLoc DL(BV);
----------------
Is this correct? The previous behaviour used `PreExtendType` directly as an operand to `DAG.getAnyExtOrTrunc` (which kept it aligned with `PreExtendVT`), but this clamps it to a smallest type of i32.


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

https://reviews.llvm.org/D120018