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

[Dave Green via Phabricator via llvm-commits]

dmgreen added subscribers: samparker, dmgreen.
dmgreen added a comment.

Sam has been looking at extending masked loads and stores in D68337 <https://reviews.llvm.org/D68337> and related patches. There looks like there would be some overlap with this, especially in the target independent parts. Make sure you co-ordinate with him.



================
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);
----------------
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.


================
Comment at: llvm/lib/Target/AArch64/AArch64SVEInstrInfo.td:1077
+
+    def _default_z : Pat<(Ty (Load  GPR64:$base, (PredTy PPR:$gp), (SVEUndef))),
+                         (RegImmInst PPR:$gp, GPR64:$base, (i64 0))>;
----------------
What if the passthru isn't undef?


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


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


================
Comment at: llvm/lib/Target/AArch64/SVEInstrFormats.td:296
 
+def SVEUndef : ComplexPattern<i64, 0, "SelectUndef", []>;
+
----------------
Can this just use "undef"?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68877