[PATCH] D133421: [AArch64] break non-temporal loads over 256 into 256-loads and a smaller load

[Florian Hahn via Phabricator via llvm-commits]

fhahn accepted this revision.
fhahn added a comment.
This revision is now accepted and ready to land.

LGTM, thanks for the latest changes. Some small stylistic comments inline.



================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:17824
 
+// Break up nontermporal larger than 256-bits loads so LDNPQ 256-bit load
+// instruction can be selected
----------------
nit: I think this is easier to read if you keep `nontemporal loads` together. Also, period at end of sentence.


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:17850
+  // Replace any non temporal load over 256-bit with a series of 256 bit loads
+  // and a scalar/vector load less than 256. This way we can utilise 256-bit
+  // loads and reduce the amount of load instructions generated.
----------------
nit: utilise -> utilize (American spelling)


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:17867
+    LoadOpsChain.push_back(SDValue(cast<SDNode>(NewLoad), 1));
+  }
+  // Process remaining bits of the load operation.
----------------
nit: add newline after here, to separate blocks.


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:17869
+  // Process remaining bits of the load operation.
+  // This is done by creating a null value vector to match the size of the
+  // 256-bit loads and inserting the remaining load to it. We extract the
----------------
nit: UNDEF vector instead of `null value vector`?


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:17891
+  LoadOpsChain.push_back(SDValue(cast<SDNode>(RemainingLoad), 1));
+  EVT ConcatVectorType =
+      EVT::getVectorVT(*DAG.getContext(), MemVT.getScalarType(),
----------------
nit: use `ConcatVT` to match other names here.


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:17904
+}
 static SDValue performSTORECombine(SDNode *N,
                                    TargetLowering::DAGCombinerInfo &DCI,
----------------
add newline before the new function/


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133421