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

[Eli Friedman via Phabricator via llvm-commits]

efriedma marked 2 inline comments as done.
efriedma added inline comments.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp:3917
+      TypeSize ElementSize = DL->getTypeAllocSize(GTI.getIndexedType());
+      // We intentionally mask away the high bits here; ElementSize may not
+      // fit in IdxTy.
----------------
sdesmalen wrote:
> I'm not sure I understand this comment; why would ElementSize not fit into IdxTy?
If you're on a 32-bit target, and you have a GEP over a type larger than 4GB.  (Not sure this is something that *should* be supported, but we have at least one testcase in the LLVM tree.)


================
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
----------------
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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73602