[PATCH] D85138: [ARM][MVE] Enable tail predication for loops containing MVE gather/scatters

[Dave Green via Phabricator via llvm-commits]

dmgreen added a comment.

I was looking at this pass for something else recently. I'm not sure if the legality checks are really necessary in here. We might want to change it to just check for active lane masks and convert them to vctp's (when legal). I think that will always be better in terms of code quality than the expansion of the active lane mask, no matter if we end up transforming to a tail predicated loop or not.

That's not really relevant for this patch though, which looks OK as far as I can tell.



================
Comment at: llvm/lib/Target/ARM/MVETailPredication.cpp:156
   Intrinsic::ID ID = Call->getIntrinsicID();
   // TODO: Support gather/scatter expand/compress operations.
+  return ID == Intrinsic::masked_store || ID == Intrinsic::masked_load ||
----------------
This comment can be removed now.


================
Comment at: llvm/lib/Target/ARM/MVETailPredication.cpp:237-251
+  unsigned ID = I->getIntrinsicID();
+  FixedVectorType *VecTy;
+  if (ID == Intrinsic::masked_load || ID == Intrinsic::masked_store) {
+    unsigned TypeOp = (ID == Intrinsic::masked_load) ? 0 : 1;
+    auto *PtrTy = cast<PointerType>(I->getOperand(TypeOp)->getType());
+    VecTy = dyn_cast<FixedVectorType>(PtrTy->getElementType());
+  } else if (ID == Intrinsic::masked_scatter ||
----------------
Can this entire function use:
 - The return type of a load/gather
 - The value type of a store/scatter
?


================
Comment at: llvm/lib/Target/ARM/MVETailPredication.cpp:243
+    VecTy = dyn_cast<FixedVectorType>(PtrTy->getElementType());
+  } else if (ID == Intrinsic::masked_scatter ||
+             ID == Intrinsic::arm_mve_vldr_gather_offset ||
----------------
I don't think this pass should ever see valid Intrinsic::masked_scatter's that have not been transformed into some form of Intrinsic::arm_mve_vstr_...


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

https://reviews.llvm.org/D85138