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

[Florian Hahn via Phabricator via llvm-commits]

fhahn added a comment.

The updated metadata looks fine and should be flexible enough to cover the scenarios outlined by @paulwalker-arm, whatever the clang support looks like. A few small remaining comments.



================
Comment at: llvm/include/llvm/Transforms/Utils/LoopUtils.h:217
+llvm::Optional<ElementCount>
+getOptionalElementCountLoopAttribute(Loop *TheLoop);
+
----------------
It this is exposed here, this should probably have a comment describing what this does?


================
Comment at: llvm/lib/Transforms/Utils/LoopUtils.cpp:306
+llvm::getOptionalElementCountLoopAttribute(Loop *TheLoop) {
+  const MDOperand *AttrMD =
+      findStringMetadataForLoop(TheLoop, "llvm.loop.vectorize.width")
----------------
Here we get both the integer values of `llvm.loop.vectorize.width` and `llvm.loop.vectorize.scalable.enable"`, right? Could this be simplified by using the existing `getOptionalIntLoopAttribute`?


================
Comment at: llvm/test/Transforms/LoopVectorize/no_array_bounds_scalable.ll:65
+
+attributes #0 = { nofree norecurse nounwind "disable-tail-calls"="false" "frame-pointer"="non-leaf" "less-precise-fpmad"="false" "min-legal-vector-width"="0" "no-infs-fp-math"="false" "no-jump-tables"="false" "no-nans-fp-math"="false" "no-signed-zeros-fp-math"="false" "no-trapping-math"="true" "stack-protector-buffer-size"="8" "target-cpu"="generic" "target-features"="+neon" "unsafe-fp-math"="false" "use-soft-float"="false" }
+
----------------
SjoerdMeijer wrote:
> nit: we probably don't need all of this.
looks like the TBAA metadata still can be removed?


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

https://reviews.llvm.org/D88962