[PATCH] D71773: [AArch64][SVE] Update the definition of AdvSIMD_GatherLoad_VecTorBase_Intrinsic

[Sander de Smalen via Phabricator via llvm-commits]

sdesmalen added inline comments.


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:12219
+
+  // GLD1_IMM requires that the offset is an immediate in the range 0-31. For
+  // immediates outside that range and non-immediate scalar offsets use GLD1 or
----------------
efriedma wrote:
> "GLD1_IMM requires that the offset is an immediate in the range 0-31" is not correct, in general; the immediate is multiplied by the element size.
This code should not interpret the offset as a byte offset and then scale the offset limit, but rather scale the offset with the element size before swapping Base and Offset.

Perhaps we should rename the intrinsic to `llvm.aarch64.sve.ld1.gather.scalar.index` to clarify the distinction between it being a scaled and unscaled offset (this also gives it the same name as in the ACLE and the architecture spec).


================
Comment at: llvm/test/CodeGen/AArch64/sve-intrinsics-gather-loads-vector-base-imm-offset.ll:178
+; CHECK-LABEL: gld1b_s_imm_offset_oor:
+; CHECK: mov	w8, #32
+; CHECK-NEXT: ld1b { z0.s }, p0/z, [x8, z0.s, uxtw]
----------------
this needs to be a `mov w8, #128` (scaled by the element size, 4) instead.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71773