[PATCH] D95870: [RISCV] Support the scalable-vector fadd reduction intrinsic

[Craig Topper via Phabricator via llvm-commits]

craig.topper added inline comments.


================
Comment at: llvm/lib/Target/RISCV/RISCVISelLowering.cpp:1718
+  SDLoc DL(Op);
+  assert(Op.getValueType().isSimple() && "Unexpected vector-reduce lowering");
+  MVT VecEltVT = Op.getValueType().getSimpleVT();
----------------
Won't getSimpleVT) below already assert?


================
Comment at: llvm/lib/Target/RISCV/RISCVISelLowering.cpp:1719
+  assert(Op.getValueType().isSimple() && "Unexpected vector-reduce lowering");
+  MVT VecEltVT = Op.getValueType().getSimpleVT();
+  // We have to perform a bit of a dance to get from our vector type to the
----------------
getSimpleValueType()


================
Comment at: llvm/lib/Target/RISCV/RISCVISelLowering.cpp:1731
+  SDValue ZeroIdx = DAG.getConstant(0, DL, Subtarget.getXLenVT());
+  SDValue ScalarInVec = DAG.getNode(ISD::INSERT_VECTOR_ELT, DL, M1VT,
+                                    DAG.getUNDEF(M1VT), ScalarVal, ZeroIdx);
----------------
I think ISD::SCALAR_TO_VECTOR is what we would more commonly use.


================
Comment at: llvm/test/CodeGen/RISCV/rvv/vreductions-fp-sdnode.ll:31
+; RV64-NEXT:    vfmv.f.s ft0, v25
+; RV64-NEXT:    fadd.h fa0, fa0, ft0
+; RV64-NEXT:    ret
----------------
Annoying that SelectionDAG breaks the scalar input out separately.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95870