[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/lib/Target/AArch64/AArch64ISelLowering.cpp:12914
 
+/// Combine the gather prefetch (scalar + vector addressing mode) when
+/// the offset vector is an unpacked 32-bit scalable vector.
----------------
nit: this comment is a bit confusing. It doesn't so much 'combine' anything, but it rather 'legalizes the offset'.


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:12918
+                                                            SelectionDAG &DAG) {
+  const SDValue Offset = N->getOperand(3);
+
----------------
nit: unnecessary use of `const` (same for other places in this patch).


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:12984
+  SDValue Ops[] = {
+      N->getOperand(0),                      // Chain
+      DAG.getConstant(NewIID, DL, MVT::i64), // Intrinsic ID
----------------
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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75580