[PATCH] D88962: [SVE] Add support for scalable vectors with vectorize.scalable.enable loop attribute

Mon Nov 30 01:01:03 PST 2020

fhahn accepted this revision.
fhahn added a comment.
This revision is now accepted and ready to land.

LGTM, thanks. Given that there has been quite some discussion, it would be good to wait with landing this for a few days, in case there are any more comments.

================
Comment at: llvm/include/llvm/Transforms/Utils/LoopUtils.h:220
+/// then None is returned.
+llvm::Optional<ElementCount>
+getOptionalElementCountLoopAttribute(Loop *TheLoop);
----------------
nit: drop `llvm::` (there are some inconsistencies about that in the file, but it should not be required and it is also not used for other things in the `llvm` namespace, like `Loop`).

================
Comment at: llvm/lib/Transforms/Utils/LoopUtils.cpp:475
   // this tranformation.
-  if (Enable == true && VectorizeWidth == 1 && InterleaveCount == 1)
+  if (Enable == true && (VectorizeWidth && VectorizeWidth->isScalar()) &&
+      InterleaveCount == 1)
----------------
nit: the extra `()` are not needed?

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

https://reviews.llvm.org/D88962