[PATCH] D77435: [llvm][CodeGen] Addressing modes for SVE stN.

[Cullen Rhodes via Phabricator via llvm-commits]

c-rhodes added a comment.

@fpetrogalli thanks for patch. Couple of nits that can be addressed before merging, no need to update this patch. LGTM!



================
Comment at: llvm/lib/Target/AArch64/AArch64ISelDAGToDAG.cpp:1425
+  const bool IsRegImm =
+      SelectAddrModeIndexedSVE<-8 /*Min*/, 7 /*Max*/>(N, Base, Base, Offset);
+
----------------
fpetrogalli wrote:
> c-rhodes wrote:
> > c-rhodes wrote:
> > > I think comments with var names are usually written like: `/*Min=*/-8, /*Max=*/7`
> > The parameters here are a bit confusing, `N` is passed as `Root` and `Base` for `N` and `Base`? Are both `N` and `Base` necessary or could this be refactored?
> `Root` and `N` need to be passed because of the way the ComplexPatterns in tablegen require to define such methods when defining the addressing modes. To make it more explicit what is in input and what is in output in this method, I have changed its signature to return the opcode, the new optimized base and offset as a tuple. The names of the variables should now make it clear what it in input and what is in output.
thanks for clearing it up!


================
Comment at: llvm/test/CodeGen/AArch64/sve-intrinsics-stN-reg-imm-addr-mode.ll:246
+; CHECK-LABEL: st3b_i8_valid_imm_lower_bound:
+; CHECK:      st3b { z0.b, z1.b, z2.b }, p0, [x0, #-24, mul vl]
+; CHECK-NEXT: ret
----------------
fpetrogalli wrote:
> c-rhodes wrote:
> > nit: formatting
> Not sure what you want me to do here.
I don't think I do either :D

I noticed in the valid upper/lower bound tests you have:
```; CHECK:      st3b { z0.b, z1.b, z2.b }, p0, [x0, #-24, mul vl]
; CHECK-NEXT: ret```

and others (e.g. `st3h_i16`):
```; CHECK: st3h { z0.h, z1.h, z2.h }, p0, [x0, #6, mul vl]
; CHECK-NEXT: ret```

but I know that wasn't what I meant yesterday, feel free to ignore this.



================
Comment at: llvm/test/CodeGen/AArch64/sve-intrinsics-stN-reg-imm-addr-mode.ll:225
+                                          <vscale x 16 x i8> %v1,
+                                          <vscale x 16 x i8> %v2,<vscale x 16 x i1> %pred,
+                                          <vscale x 16 x i8>* %base)
----------------
nit: move pred next line


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77435