[PATCH] D68877: [AArch64][SVE] Implement masked load intrinsics

[Kerry McLaughlin via Phabricator via cfe-commits]

kmclaughlin marked 4 inline comments as done.
kmclaughlin added a comment.

Thanks for reviewing this, @dmgreen! I have updated the patch to make use of the changes to DAGCombine introduced by D68337 <https://reviews.llvm.org/D68337>.



================
Comment at: llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp:10393
+      ((!LegalOperations && !cast<MaskedLoadSDNode>(N0)->isVolatile()) ||
+       TLI.isLoadExtLegal(ISD::SEXTLOAD, VT, EVT))) {
+    MaskedLoadSDNode *LN0 = cast<MaskedLoadSDNode>(N0);
----------------
dmgreen wrote:
> I'm not convinced that just because a sext load is legal and a masked load is legal, that a sext masked load is always legal.
Removed, as this patch can also use tryToFoldExtOfMaskedLoad for sext & zext masked loads.


================
Comment at: llvm/lib/Target/AArch64/AArch64TargetTransformInfo.h:151
+  bool isLegalMaskedLoad(Type *DataType) {
+    return ST->hasSVE();
+  }
----------------
dmgreen wrote:
> This can handle all masked loads? Of any type, extended into any other type, with any alignment?
I have added checks on types we can handle here


================
Comment at: llvm/lib/Target/AArch64/AArch64TargetTransformInfo.h:153
+  }
+  bool isLegalMaskedStore(Type *DataType) {
+    return ST->hasSVE();
----------------
dmgreen wrote:
> This patch doesn't handle stores yet.
Removed!


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

https://reviews.llvm.org/D68877