[PATCH] D151221: [RISCV] Scalarize constant stores of fixed vectors up to 32 bits

[Philip Reames via Phabricator via llvm-commits]

reames requested changes to this revision.
reames added inline comments.
This revision now requires changes to proceed.


================
Comment at: llvm/lib/Target/RISCV/RISCVISelLowering.cpp:12190
     // initialize any power-of-two size up to XLen bits.
     if (DCI.isBeforeLegalize() && IsScalarizable &&
         ISD::isBuildVectorAllZeros(Val.getNode())) {
----------------
Isn't this case now fully covered by the one you added?  I think you can delete this.  

Ah, I see the problem.  Rather than checking that MemVT is less than 32 bits, you can check that the splat constant isUint<32>.  

Or if you want, you can directly check materialization cost < 2 using RISCVMatInt.cpp/h


================
Comment at: llvm/lib/Target/RISCV/RISCVISelLowering.cpp:12219
+          MemVT.getVectorElementType().bitsGE(MVT::i8) &&
+          cast<BuildVectorSDNode>(Val)->isConstantSplat(
+              SplatVal, SplatUndef, SplatBitSize, HasAnyUndefs,
----------------
isConstantSplat seems like an odd interface here.  I looked for something obviously better and didn't find it though...


================
Comment at: llvm/test/CodeGen/RISCV/rvv/fixed-vectors-int-buildvec.ll:66
 
 define void @buildvec_vid_step2_add0_v4i8(ptr %z0, ptr %z1, ptr %z2, ptr %z3) {
+; RV32-LABEL: buildvec_vid_step2_add0_v4i8:
----------------
I think you're loosing the spirit of the test on these.  This looks to be intended to exercise materialization of the vector constant, not the store of that constant to memory.  Can you adjust the test to keep the prior codegen or something close to it?

You can return the values from individual tests, or use volatile stores.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151221