[PATCH] D73602: [SVE] Add support for lowering GEPs involving scalable vectors.

[Francesco Petrogalli via Phabricator via llvm-commits]

fpetrogalli marked an inline comment as done.
fpetrogalli added inline comments.


================
Comment at: llvm/test/CodeGen/AArch64/sve-gep.ll:7-8
+; CHECK:       // %bb.0:
+; CHECK-NEXT:    rdvl x8, #1
+; CHECK-NEXT:    add x0, x0, x8, lsl #2
+; CHECK-NEXT:    ret
----------------
efriedma wrote:
> fpetrogalli wrote:
> > These two instructions could be replaced by a single one: `addvl x0, x0, #4`. Do you think you can change your patch to generate this sequence, or should this be done in the back-end?
> > 
> > Also, the code I wrote in the patch at https://reviews.llvm.org/D74254 generates the following node for the base address out of this example (well, my patches uses size in bits, but I recon it should be bytes):
> > 
> > ```
> > (ISD::ADD, %base (ISD::VSCALE 16))
> > ```
> > 
> > It essentially encodes the multiplicative constant in the VSCALE node directly (provided that the argument of the MUL is an immediate constant). That makes it very easy to detect in the back-end because it doesn't require to check for MUL/SUB and SHL. Do you think you can emit such DAG in your patch? 
> We probably need to add patterns for "add(x, vscale) -> addvl" in TableGen; that's not really related to this patch, though.
> 
> I guess I could modify the current patch where it does `if (CI && !ElementScalable) {` to generate an appropriate vscale node directly.  But we probably want a `mul(vscale(n1), n2) -> vscale(n1*n2)` dagcombine anyway.
> We probably need to add patterns for "add(x, vscale) -> addvl" in TableGen; that's not really related to this patch, though.

OK

> I guess I could modify the current patch where it does if (CI && !ElementScalable) { to generate an appropriate vscale node directly. But we probably want a mul(vscale(n1), n2) -> vscale(n1*n2) dagcombine anyway.

Ah, DAG Combine. Right. Yep, we can do that there.


================
Comment at: llvm/test/CodeGen/AArch64/sve-gep.ll:42
+; CHECK-NEXT:    ret
+  %d = getelementptr <vscale x 2 x i64>, <vscale x 2 x i64>* %base, <2 x i64> <i64 1, i64 1>
+  ret <2 x <vscale x 2 x i64>*> %d
----------------
Maybe try two different constants in the `<2 x i64>` vector? And also a version with one of the elements of the vector coming from an input variable of the function?


================
Comment at: llvm/test/CodeGen/AArch64/sve-gep.ll:53
+; CHECK-NEXT:    ret
+  %d = getelementptr <vscale x 2 x i64>, <2 x <vscale x 2 x i64>*> %base, <2 x i64> <i64 1, i64 1>
+  ret <2 x <vscale x 2 x i64>*> %d
----------------
Same here. Worth using two different constants and also a version in which the elements of the index vector are not constant but runtime values.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73602