[PATCH] D74858: [AArch64][SVE] Add intrinsics for non-temporal gather-loads/scatter-stores

[Andrzej Warzynski via Phabricator via llvm-commits]

andwar marked 3 inline comments as done.
andwar added a comment.

In D74858#1886601 <https://reviews.llvm.org/D74858#1886601>, @sdesmalen wrote:

> This patch is doing a lot of refactoring/renaming, can you separate that out into a separate NFC patch?


Extracted and submitted here: https://reviews.llvm.org/rGcff90c938b7be43de482ffb7a8a7fdbdf57c32a3

> On the point for having separate intrinsics for (vector, scalar) and (scalar, vector), I think this makes sense, as we'll then use similar intrinsics for ldnt1 and ld1 and we can target the right instruction in a > similar way we do this in performLD1GatherCombine.

I've updated this patch accordingly. I've kept the names of the new intrinsics consistent with the regular gathers/scatters. The intrinsics added here should be sufficient to support what's defined in ACLE, but there's still fewer LLVM IR intrinsics for non-temporal scatters/gathers than there is for regular gathers/scatters. Note that for the (scalar, vector) case I added 2 intrinsics:

- `@llvm.aarch64.sve.ldnt1.gather` - 32bit wide offsets
- `@llvm.aarch64.sve.ldnt1.gather.uxtw` - 64bit wide offsets

Doing it this way makes it more natural to re-use the exisiting code for regular gathers/scatters and keeps things consistent. I hope that this makes sense.



================
Comment at: llvm/lib/Target/AArch64/SVEInstrFormats.td:4947
 
-multiclass sve2_mem_sstnt_vs<bits<3> opc, string asm,
-                             RegisterOperand listty, ZPRRegOp zprty> {
-  def _REAL : sve2_mem_sstnt_vs_base<opc, asm, listty, zprty>;
+multiclass sve2_mem_sstnt_32b_ptrs<bits<3> opc, string asm,
+                             SDPatternOperator op,
----------------
sdesmalen wrote:
> nit: vec_32b_ptrs ?
I'll use `vec_32_ptrs` instead. That's consistent with regular gather loads: https://github.com/llvm/llvm-project/blob/cff90c938b7be43de482ffb7a8a7fdbdf57c32a3/llvm/lib/Target/AArch64/SVEInstrFormats.td#L6257


================
Comment at: llvm/lib/Target/AArch64/SVEInstrFormats.td:4950
+                             ValueType vt> {
+  def _REAL : sve2_mem_sstnt_vs_base<opc, asm, Z_s, ZPR32>;
 
----------------
sdesmalen wrote:
> If you change the name of this class, you may want to update the parent class as well.
What about setting the name of this multi class to `sve2_mem_gldnt_vec_vs_32_ptrs` instead? This way updating the name of the base class is no longer needed. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74858