[PATCH] D94593: [SVE] Restrict the usage of REINTERPRET_CAST.

[David Sherwood via Phabricator via llvm-commits]

david-arm added inline comments.


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:17197
+  // Pack input if required.
+  if (InVT != PackedInVT)
+    Op = DAG.getNode(AArch64ISD::REINTERPRET_CAST, DL, PackedInVT, Op);
----------------
If InVT is a float vector then getPackedSVEVectorVT will return an integer vector and you'll have a reinterpret_cast that you don't want, right? It will fail to select, but I wonder if you want an explicit assert for InVTs only supporting integer types at the moment?


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:17203
+  // Unpack result if required.
+  if (VT != PackedVT)
+    Op = DAG.getNode(AArch64ISD::REINTERPRET_CAST, DL, VT, Op);
----------------
I'm not sure how this works for the case we recently saw where a Gather result type was nxv2i64 and VT was nxv2f16? In this case InVT will equal PackedInVT, we'll bitcast Op (already nx2vi64) to nxv2i64 again, then do a reinterpret_cast from Op (nxv2i64) to v2f16. Unless I'm missing something?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94593