[PATCH] D79587: [CodeGen][SVE] Legalisation of extends with scalable types

[Kerry McLaughlin via Phabricator via cfe-commits]

kmclaughlin marked 2 inline comments as done.
kmclaughlin added a comment.

Thanks for taking another look at this, @efriedma!



================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:10676
+    ConstantSDNode *C = dyn_cast<ConstantSDNode>(Dup->getOperand(0));
+    uint64_t ExtVal = C->getZExtValue();
+
----------------
efriedma wrote:
> Do you need to truncate ExtVal somewhere, so you don't end up with a DUP with an over-wide constant?
I've changed the call to `getNode` below that creates the DUP to use `DAG.getAnyExtOrTrunc` (similar to what we do in LowerSPLAT_VECTOR)


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:13300
+    // is another unpack:
+    // 4i32 sign_extend_inreg (4i32 uunpklo(8i16 uunpklo (16i8 opnd)), from 4i8)
+    // ->
----------------
efriedma wrote:
> It seems a little fragile to assume the inner VT of the SIGN_EXTEND_INREG is exactly the type you're expecting here.  Probably worth at least adding an assertion to encode the assumptions you're making.
I've added an assert above here to make sure the sign_extend_inreg and unpack types match, is this the assumption you were referring to?


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:13816
+  if (!isTypeLegal(InVT)) {
+    // Bubble truncates to illegal types to the surface.
+    if (In->getOpcode() == ISD::TRUNCATE) {
----------------
efriedma wrote:
> "Bubble truncates to illegal types to the surface" is an optimization?
Removed - this was not required for this patch.


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

https://reviews.llvm.org/D79587