[PATCH] D158854: [RISCV] Move vmv_s_x and vfmv_s_f special casing to DAG combine

Philip Reames via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Aug 25 10:39:49 PDT 2023


reames added inline comments.


================
Comment at: llvm/lib/Target/RISCV/RISCVISelLowering.cpp:13948
+        Const && !isNullConstant(Scalar) && isInt<5>(Const->getSExtValue()) &&
+        VT.bitsLE(getLMUL1VT(VT)) && Passthru.isUndef())
+      return DAG.getNode(RISCVISD::VMV_V_X_VL, DL, VT, Passthru, Scalar, VL);
----------------
luke wrote:
> So if I'm understanding correctly, the reason for the extra test cases being picked up is because the combine above to shrink LMUL down to 1 runs first, and then this combine is run because now LMUL=1. 
> Can we get rid of `VT.bitsLE(getLMUL1VT(VT))` condition and comment here? Since the above combine will always run first in this case.
Er, I think you misunderstood.  These are independent transforms.  The test change comes from the fact we're generating a whole bunch of vmv_s_x/f directly without going through the lowerScalarInsert cover function.  To my knowledge, there's no order of transform issue here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158854



More information about the llvm-commits mailing list