[PATCH] D69845: [ARM][MVE] canTailPredicateLoop

[Sam Parker via Phabricator via llvm-commits]

samparker added inline comments.


================
Comment at: llvm/lib/Target/ARM/ARMTargetTransformInfo.cpp:1010
+  if (isa<TruncInst>(&I) || isa<SExtInst>(&I) || isa<ZExtInst>(&I)) {
+    if (isa<LoadInst>(I.getOperand(0)) || isa<StoreInst>(I.getOperand(0))) {
+      LLVM_DEBUG(dbgs() << "not allowing instruction: "; I.dump());
----------------
You need to check the user of the load, not it's operand. Also the logic is wrong here, we need to accept extending loads and truncating stores.


================
Comment at: llvm/lib/Target/ARM/ARMTargetTransformInfo.cpp:1050
+      // TODO: the float types
+      if (T->isHalfTy() || T->isFloatTy() || T->isDoubleTy())
+        return false;
----------------
What is the complication of supporting half and float now?


================
Comment at: llvm/lib/Target/ARM/ARMTargetTransformInfo.cpp:1064
+        // keep it simple for now.
+        if (FirstStride && (NextStride == 1 || NextStride == -1)) {
+          FirstStride = false;
----------------
You don't need FirstStride, the uninitialised Stride holds the same info.


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

https://reviews.llvm.org/D69845