[PATCH] D101945: [LV] Add -scalable-vectorization=<option> flag.

[Vineet Kumar via Phabricator via llvm-commits]

vkmr accepted this revision.
vkmr added a comment.

LGTM.



================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorizationLegality.cpp:60
+        clEnumValN(LoopVectorizeHints::SK_FixedWidthOnly, "off",
+                   "Scalable vectorisation is disabled."),
+        clEnumValN(LoopVectorizeHints::SK_PreferFixedWidth, "on",
----------------
sdesmalen wrote:
> vkmr wrote:
> > paulwalker-arm wrote:
> > > Sorry my bad :) you probably want `s/vectorisation/vectorization/`
> > Perhaps I am bike-shedding a bit much here (and I am not too sure about this), but the "on" and "off" terminology indicates binary choice, not making it readily clear that there is a possible third choice "preferred". Also reading `-scalable-vectorization=on` doesn't make it clear that it in fact prefers fixed vectorization.
> > May be something like "off", "possible" and "preferred" is clearer. 
> > Thoughts?
> It's a good question! From my perspective, having the flag set to 'on' kind of does what it promises, it enables scalable vectorization. It doesn't really suggest to what extend it enables scalable vectorization, which leaves us some room to play with what a good default is. Whereas 'preferred' suggests that scalable vectorization is the preferred way to vectorize loops, instead of fixed-width. I guess the interpretation of 'on' isn't too dissimilar to 'possible', but while the latter is more accurate for now, the former may be more accurate going forward. Unless you feel strongly about this, I have a slight preference to keep 'on' as this feels a bit more future-proof as a default.
> 
> I'm not too worried about having much confusion about this, because `--help-hidden` shows all three options with their meaning, and it's not really meant to be a user-facing option in the sense that end-users will see it, but rather an experimental option for now.
I agree with the future-proof reasoning. I am fine with the way it is. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101945