[PATCH] D75580: [llvm][CodeGen][SVE] Implement IR intrinsics for gather prefetch.

[Francesco Petrogalli via Phabricator via llvm-commits]

fpetrogalli marked 16 inline comments as done.
fpetrogalli added a comment.

In D75580#1905227 <https://reviews.llvm.org/D75580#1905227>, @andwar wrote:

> Hi @fpetrogalli, thank you for working on this!
>
> My main points:
>
> - ACLE allows scalar indices that may be out of range for `PRFH <prfop>, <Pg>, [<Zn>.S{, #<imm>}]`. Will we be able to cater for those with your approach?


Yep, I have added a test to cover all cases in which the intrinsic cannot be lowered to such instruction (run time values or invalid immediate).

> - Could you please remove the extra empty lines from test files? (and make sure that the formatting is consistent)

Done! Please double check I haven't missed any.

> - We tend to put declarations at the end of the test files (I've not seen a test with a different style, but I've not seen them all either ;-) )

Done.



================
Comment at: llvm/lib/Target/AArch64/AArch64SVEInstrInfo.td:13
 
+class gather_prf_scaled_pat_frag<SDPatternOperator intrinsic, code predicate>
+  : PatFrag<(ops node:$base, node:$vec_offset, node:$pred, node:$prfop),
----------------
andwar wrote:
> Please, could this block) of code (and it's sunblocks) could be documented/commented consistently with the rest of the file? E.g.:
> ```
> // Prefetches - pattern definitions
> //
> ```
> Having said that, this file is growing organically so we may want to revisit the current style.
I have removed these `PatFrag`, they are not needed anymore in last patch.


================
Comment at: llvm/lib/Target/AArch64/AArch64SVEInstrInfo.td:18
+
+ def gather_prfb_scaled_uxtw
+   : gather_prf_scaled_pat_frag<int_aarch64_sve_gather_prf_scaled_uxtw,
----------------
andwar wrote:
> Why extra indentation?
PatFrags removed from the patch.


================
Comment at: llvm/test/CodeGen/AArch64/sve-intrinsics-gather-prefetches-scaled-offset.ll:132
+
+declare void @llvm.aarch64.sve.gather.prf.scaled.sxtw.p0f16.nx2vi64(half* %base, <vscale x 2 x i64> %offset, <vscale x 2 x i1> %Pg, i32 %prfop)
+define void @llvm_aarch64_sve_gather_prf_scaled_sxtw_p0f16_nx2vi64(half* %base, <vscale x 2 x i64> %offset, <vscale x 2 x i1> %Pg) {
----------------
fpetrogalli wrote:
> The offset vector here should be `<vscale x 2 x i32>`, not `<vscale x 2 x i64>`. I'll fix this.
This is now working, the offset uses the correct scalable vector `<vscale x 2 x i32>`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75580