[PATCH] D107790: [RISCV] Add a pass to recognize VLS strided loads/store from gather/scatter.

[Craig Topper via Phabricator via llvm-commits]

craig.topper added inline comments.


================
Comment at: llvm/lib/Target/RISCV/RISCVGatherScatterLowering.cpp:344
+
+  unsigned IncrementingBlock = BasePhi->getOperand(0) == Inc ? 0 : 1;
+
----------------
frasercrmck wrote:
> I think this //might// be guaranteed by loop simplify form, but here and/or in `matchStridedRecurrence` I think it might be best to double-check (or bail) if the PHI does/doesn't have exactly two operands or even that the incrementing block is what we expect.
I can assert here I guess. The 2 operand check for the Phi was done by matchSimpleRecurrence


================
Comment at: llvm/lib/Target/RISCV/RISCVGatherScatterLowering.cpp:373
+
+  auto BaseAndStride = std::make_pair(BasePtr, Stride);
+
----------------
frasercrmck wrote:
> no need for new variable?
Must be left over from when this code was inlined in tryCreateStridedLoadStore earlier in development.


================
Comment at: llvm/lib/Target/RISCV/RISCVGatherScatterLowering.cpp:444
+    for (Instruction &I : BB) {
+      // Do an initial optimization pass to push out as much address arithmetic
+      // as possible to get a more canonical IR.
----------------
frasercrmck wrote:
> Is this comment right, or in the wrong place?
Leftover from an earlier version where I did that after copying it from ARM.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107790