[PATCH] D88962: [SVE] Add support for scalable vectors in vectorize_width loop attribute

Florian Hahn via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Oct 7 07:23:56 PDT 2020


fhahn requested changes to this revision.
fhahn added a comment.
This revision now requires changes to proceed.

Could you also update the documentation for `llvm.loop.vectorize.width`? https://llvm.org/docs/LangRef.html#llvm-loop-vectorize-width-metadata



================
Comment at: llvm/include/llvm/Transforms/Vectorize/LoopVectorizationLegality.h:54
+    union {
+      unsigned U32;
+      ElementCount EC;
----------------
might be good to add comments indicating what the fields are used for.


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorizationLegality.cpp:67
+      return false;
+    const ConstantInt *C0 =
+        mdconst::dyn_extract<ConstantInt>(MD->getOperand(0));
----------------
are you anticipating additional hints with ElementCount metadata? If not, it might be simpler to just deal with HK_WIDTH up front and validate Val[0] and Val[1] here and leave the code handling the other cases mostly unchanged? Or maybe use named variables instead of an array, to make things a bit clearer?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88962



More information about the llvm-commits mailing list