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

[Sander de Smalen via Phabricator via llvm-commits]

sdesmalen added inline comments.


================
Comment at: llvm/include/llvm/IR/IntrinsicsAArch64.td:1263
+                  llvm_anyvector_ty, // Offsets
+                  LLVMScalarOrSameVectorWidth<0, llvm_i1_ty>, // Predicate
+                  llvm_i32_ty // Prfop
----------------
I realise this only now, but having the predicate as the third operand is different from the gather loads. Can you make this to be the first operand, rather than the third, to streamline their definition with the gather loads?


================
Comment at: llvm/include/llvm/IR/IntrinsicsAArch64.td:1273
+                  llvm_i64_ty, // Scalar offset
+                  LLVMScalarOrSameVectorWidth<0, llvm_i1_ty>, // Predicate
+                  llvm_i32_ty // Prfop
----------------
Same here.


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:12984
+  SDValue Ops[] = {
+      N->getOperand(0),                      // Chain
+      DAG.getConstant(NewIID, DL, MVT::i64), // Intrinsic ID
----------------
sdesmalen wrote:
> Given that you've gone the route of doing this in ISelLowering rather than having ComplexPatterns, it's probably better to create ISD nodes rather than passing the intrinsics around. This means we can later reuse them if there is ever a llvm.gather.prefetch, but also to streamline the implementation of prefetches with that of the LD1 gathers. It would also do away with having to pass the operands explicitly like this and thus simplify these combines.
We discussed this offline and decided it's fine for now to use the intrinsics directly. We may revisit this later when we clean up the logic in this file for mapping the gathers/scatters/prefetches.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75580