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

[Dave Green via Phabricator via llvm-commits]

dmgreen added a comment.

In an ideal world / in the long run, it would be better for the option of whether to predicate to be dependant on the vectorisation factor. I believe we will have cases where we can pick between a slower tail predicated loop or a quicker non-tail predicated loop, and we should be able to pick the quicker one. This implies that it should really be incorporated into the cost mechanism inside the vectoriser.

(Also the current method seems a bit fuzzy, even by vectoriser standards. It seems to essentially be trying to pre-computing what vectorisation factor it thinks will be produced, choosing whether to predicate and hoping that the vectoriser does end up picking the same factor. Without doing something it doesn't expect).

But I think you are correct that this is a sensible way forward, to get something working. We can adjust it as needed, and perhaps it will work OK for almost all cases.

It does need to option to turn it off though. I would expect that should be off by default for the moment until the backend part is in and we have verified that this is working well enough.



================
Comment at: llvm/lib/Target/ARM/ARMTargetTransformInfo.cpp:1065
+        // keep it simple for now.
+        if (Stride == 0 && (NextStride == 1 || NextStride == -1)) {
+          Stride = NextStride;
----------------
Can you explain the reasoning for adding -1 strides? I would expect them to become vrev's and vmov's, both of which will be difficult to prove in the backend (if not outright incorrect).

Are you expecting any reverse shuffles to be cancelled out?


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

https://reviews.llvm.org/D69845