[PATCH] D147678: [LegalizeTypes][AArch64] Use scalar_to_vector to eliminate bitcast

Tue Jun 13 05:24:04 PDT 2023

paulwalker-arm accepted this revision.
paulwalker-arm added a comment.

I've an extra nit but otherwise looks good.

================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:22894-22897
+  [[maybe_unused]] EVT SrcVT = Op.getValueType();
+
+  assert(SrcVT.getSizeInBits() == VT.getSizeInBits() &&
+         "Expected same size for bitcast!");
----------------
Is this necessary given it is a requirement of `ISD::BITCAST`? By which I mean you shouldn't need to manually verify the DAG is valid here? I'd expect `getNode()` to catch such errors.

================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:22902
+  SDValue CastVal = DAG.getNode(ISD::BITCAST, DL, CastVT, Vec);
+  SDValue IdxZero = DAG.getConstant(0, DL, MVT::i64);
+  Results.push_back(
----------------
Please can you use `getVectorIdxConstant` here rather than hardwiring `MVT::i64`?

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

https://reviews.llvm.org/D147678