[PATCH] D157886: [RISCV] Match strided loads with reversed indexing sequences

[Philip Reames via Phabricator via llvm-commits]

reames added inline comments.
Herald added a subscriber: sunshaoce.


================
Comment at: llvm/lib/Target/RISCV/RISCVISelLowering.cpp:12818
   Align Align = BaseLd->getAlign();
-
   for (SDValue Op : N->ops().drop_front()) {
     auto *Ld = dyn_cast<LoadSDNode>(Op);
----------------
luke wrote:
> Can we do `for (SDValue Op : N->ops())` instead to remove the need for the extra isSimple() and isNormalLoad() checks above?
We can, but we still need the BaseLd and BaseLdVT variables.  Given that, I didn't think folding only part of the checks into the loop was worthwhile. 


================
Comment at: llvm/lib/Target/RISCV/RISCVISelLowering.cpp:12833
+    SDValue Stride;
+    for (auto Idx : enumerate(Ptrs)) {
+      if (Idx.index() == 0)
----------------
luke wrote:
> Just an idea, can we use one lambda and pass in a reverse iterator of the Ptrs vector?
Not unless you know how to do a reverse enumerate with a "first element" check.  We need the index check and the index + or - one bits.  I played with this and it was the best variant I found. 


================
Comment at: llvm/test/CodeGen/RISCV/rvv/fixed-vectors-strided-load-combine.ll:497
 
 ; TODO: This is a strided load with a negative stride
 define void @reverse_strided_constant_pos_4xv2f32(ptr %x, ptr %z, i64 %s) {
----------------
luke wrote:
> We can strike off this todo
Good catch.  I need to add this to my mental checklist, it's one of my regular oops.  


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157886