[PATCH] D100549: [RISCV] Lower vector shuffles to vrgather operations

[Craig Topper via Phabricator via llvm-commits]

craig.topper added a comment.

I wonder if we could take advantage of this "(vs1[i] >= VLMAX) ? 0 : vs2[vs1[i]]" to use an OR instead of a merge in some cases. If the index type has SEW=32 or SEW=64 surely a "-1" would be above VLMAX. If were to do that it should be a different patch.



================
Comment at: llvm/lib/Target/RISCV/RISCVISelLowering.cpp:1572
+  bool SwapOps = DAG.isSplatValue(V2) && !DAG.isSplatValue(V1);
+  bool InvertMask = (IsSelect && SwapOps) || (!IsSelect && !SwapOps);
+
----------------
This might be clearer as "IsSelect == SwapOps"


================
Comment at: llvm/lib/Target/RISCV/RISCVISelLowering.cpp:1607
+    // FIXME: We could promote the index to i16 and use vrgatherei16, but that
+    // may involve vector splitting if we're already at LMUL=8.
+    return SDValue();
----------------
or if max LMUL is capped.


================
Comment at: llvm/lib/Target/RISCV/RISCVInstrInfoVVLPatterns.td:146
 
 def riscv_vrgather_vx_vl : SDNode<"RISCVISD::VRGATHER_VX_VL",
+                                  SDTypeProfile<1, 5, [SDTCisVec<0>,
----------------
Should we change the names of these to include _MASK or something? This is now difference than the other _VL ISD opcodes and many of the others were set up to match the VP SDNodes.

Alternatively, we pattern match VSELECT_VL+VRGATHER_VX_VL/VRGATHER_VV_VL? I think that's how VP is intended to work.


================
Comment at: llvm/test/CodeGen/RISCV/rvv/fixed-vectors-int-shuffles.ll:104
+; CHECK-NEXT:    vrgather.vv v25, v9, v26, v0.t
+; CHECK-NEXT:    vmv1r.v v8, v25
 ; CHECK-NEXT:    ret
----------------
Depending on how costly a vrgather is, this doesn't look like much of an improvement.

It's also only using 1 element from vector 2. Would that be cheaper to just extract+insert into place?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100549