[PATCH] D89031: [SVE] Add support to vectorize_width loop pragma for scalable vectors

[Sander de Smalen via Phabricator via cfe-commits]

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

LGTM



================
Comment at: clang/lib/Sema/SemaStmtAttr.cpp:144
+      assert(ValueExpr && "Attribute must have a valid value expression.");
+      if (S.CheckLoopHintExpr(ValueExpr, St->getBeginLoc()))
+        return nullptr;
----------------
david-arm wrote:
> fhahn wrote:
> > Is there a way to only accept `fixed_width/scalable` for targets that support it? Not sure if we have enough information here, but we might be able to reject it eg per target basis or something
> Hi @fhahn, I think if possible we'd prefer not to reject scalable vectors at this point. Theoretically there is no reason why we can't perform scalable vectorisation for targets that don't have hardware support for scalable vectors. In this case it simply means that vscale is 1. If you want we could add some kind of opt-remark in the vectoriser that says something like "target does not support scalable vectors, vectorising for vscale=1"?
I agree with @david-arm that we shouldn't prevent this in the front-end. Even if the architecture may not support scalable vectors natively, there may still be reasons to want to create scalable vectors in IR, for example to have more portable IR.


================
Comment at: clang/test/CodeGenCXX/pragma-loop.cpp:166
+#pragma clang loop vectorize_width(16, fixed) interleave_count(4) unroll(disable) distribute(disable)
+  do {
+    // CHECK: br i1 {{.*}}, label {{.*}}, label {{.*}}, !llvm.loop ![[LOOP_15:.*]]
----------------
out of curiosity, is there a particular reason you're testing it with a do-while loop instead of a shorter for-loop like the tests in `for_template_constant_expression_test` ?


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

https://reviews.llvm.org/D89031