[PATCH] D130013: [AArch64][SVE] Add DAG-Combine to push bitcasts from floating point loads after DUPLANE128

[Paul Walker via Phabricator via llvm-commits]

paulwalker-arm added inline comments.


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:19212-19213
+
+  auto IdxInsert = cast<ConstantSDNode>(Insert.getOperand(2))->getZExtValue();
+  auto IdxDupLane = cast<ConstantSDNode>(N->getOperand(1))->getZExtValue();
+  if (IdxInsert != IdxDupLane)
----------------
You can use `getConstantOperandVal` here. 


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:19217
+
+  SDValue Bitcast = Insert.getOperand(1);
+  SDValue Load = Bitcast.getOperand(0);
----------------
You need to check this is in fact a `ISD::BITCAST` before accessing its operands.


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:19218
+  SDValue Bitcast = Insert.getOperand(1);
+  SDValue Load = Bitcast.getOperand(0);
+  EVT LoadVT = Load.getValueType();
----------------
I guess this is a legacy from the previous patch but can you pick a different name.


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:19220
+  EVT LoadVT = Load.getValueType();
+  if (!LoadVT.isFixedLengthVector())
+    return SDValue();
----------------
Specially this wants to be `is128BitVector()` because that's the only fixed length vector type supported for the DUPLANE128 operation.


================
Comment at: llvm/test/CodeGen/AArch64/sve-ld1r.ll:822-835
+define dso_local <vscale x 2 x double> @dupq_ld1rqd_f64() {
+; CHECK-LABEL: dupq_ld1rqd_f64:
+; CHECK:       // %bb.0:
+; CHECK-NEXT:    adrp x8, .LCPI61_0
+; CHECK-NEXT:    add x8, x8, :lo12:.LCPI61_0
+; CHECK-NEXT:    ptrue p0.d
+; CHECK-NEXT:    ld1rqd { z0.d }, p0/z, [x8]
----------------
As per our conclusion on the precursor patch, these new tests don't offer any value?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130013