[PATCH] D144175: [RISCV] Combine (store/load interleave, deinterleave) into vsseg2/vlseg2

[Craig Topper via Phabricator via llvm-commits]

craig.topper added inline comments.


================
Comment at: llvm/lib/Target/RISCV/RISCVISelLowering.cpp:10772
+      SDLoc DL(Load);
+      MVT VecVT = Load->getSimpleValueType(0);
+      MVT SubVecVT = N->getSimpleValueType(0);
----------------
How have we guaranteed the load has an MVT type?


================
Comment at: llvm/lib/Target/RISCV/RISCVISelLowering.cpp:10775
+      // Don't try to combine loads of elements that don't have a valid SEW
+      if (!RISCVVType::isValidSEW(VecVT.getScalarSizeInBits()))
+        break;
----------------
This isn't enough to guarantee the type is legal. DAG combine has to be very careful with types.


================
Comment at: llvm/lib/Target/RISCV/RISCVISelLowering.cpp:10781
+      SDValue Passthru = DAG.getUNDEF(VecVT);
+      SDValue Ops[] = {Load->getChain(),   IntID, Passthru, Passthru,
+                       Load->getBasePtr(), VL};
----------------
I think we need to check isNormalLoad for the load to make sure it isn't an extending or indexed load. Probably a good idea to check isSimple too. Especially since you aren't checking that this is the only use of the load. Same applies for stores


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144175