[PATCH] D98250: [RISCV] Optimize INSERT_VECTOR_ELT sequences

[Craig Topper via Phabricator via llvm-commits]

craig.topper added inline comments.


================
Comment at: llvm/lib/Target/RISCV/RISCVISelLowering.cpp:2223
+    SDValue One = DAG.getConstant(1, DL, XLenVT);
+    ValLo = DAG.getNode(ISD::EXTRACT_ELEMENT, DL, MVT::i32, Val, Zero);
+    ValHi = DAG.getNode(ISD::EXTRACT_ELEMENT, DL, MVT::i32, Val, One);
----------------
Would it be simpler to just check that Val is a ConstantSDNode that fits in sign extended 32 bits and if it is just call getConstant with the lower 32 bits?


================
Comment at: llvm/lib/Target/RISCV/RISCVISelLowering.cpp:2259
+    SDValue InsertI64VL = DAG.getConstant(2, DL, XLenVT);
+    // TODO: Should we be able to use UNDEF here? It appears to break the
+    // earlyclobber constraint. Just splat a zero value for now.
----------------
I can believe that doesn't work. Undef sources that aren't tied don't seem to really allocate and just pick the first register.

I'm now a little worried someone might try that in the intrinsic interface.


================
Comment at: llvm/lib/Target/RISCV/RISCVISelLowering.cpp:2267
+    ValInVec = DAG.getNode(RISCVISD::VSLIDE1UP_VL, DL, I32ContainerVT, ValInVec,
+                           ValLo, I32Mask, InsertI64VL);
+  }
----------------
Don't we need a bitcast here to get back to the ContainerVT to match the SDTypeProfile for the next operation?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98250