[PATCH] D77251: [llvm][CodeGen] Addressing modes for SVE ldN.

[Francesco Petrogalli via Phabricator via llvm-commits]

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


================
Comment at: llvm/test/CodeGen/AArch64/sve-intrinsics-ldN-reg+imm-addr-mode.ll:452
+; CHECK-NEXT: ret
+%base = getelementptr <vscale x 2 x i64>, <vscale x 2 x i64>* %addr, i64 16
+%base_ptr = bitcast <vscale x 2 x i64>* %base to i64 *
----------------
sdesmalen wrote:
> fpetrogalli wrote:
> > sdesmalen wrote:
> > > nit: when testing these pairs, can you at least make sure to test the min/max values?
> > I can do that, just making sure you didn't miss the comment on the top of file that explains why I haven't add range checks for all cases:
> > 
> > ```
> > ; NOTE: invalid, upper and lower bound immediate values of the regimm
> > ; addressing mode are checked only for the byte version of each
> > ; instruction (`ld<N>b`), as the code for detecting the immediate is
> > ; common to all instructions, and varies only for the number of
> > ; elements of the structure store, which is <N> = 2, 3, 4.
> > ```
> > 
> > Let me know if you think this is not a sufficient explanation!
> > 
> I meant using `#-32` and `#28` instead of `#16` and `#-16` for ld4.nxv8i64 and nxv8f64 respectively.
> (and similar for other tests in this file)
Sorry, I don't understand why -32/28 would be better than -16/16. The values you are suggesting are tested to be lower and upper bound for `ld4<T>` in the byte case, `ld4b`, between lines 305-323.

The range of the immediate does not depend on the scalar data type being loaded, but only on the number of vector loaded by the instruction, so I think we are goo in testing it only for `ld<N>b`. 

Or am I missing something?




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77251