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

Sander de Smalen via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon May 17 00:42:06 PDT 2021


sdesmalen added inline comments.


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorizationLegality.cpp:74
 
-bool LoopVectorizeHints::Hint::validate(unsigned Val) {
+bool LoopVectorizeHints::Hint::validate(unsigned &Val) {
   switch (Kind) {
----------------
paulwalker-arm wrote:
> I realise you've changed this in response to a previous comment but having a validation function change the value it's validating seems wrong.  You could change the function name or perhaps return an optional or something but given the enum values have been renamed I wonder if the original comment has been nullified?
> 
> That's to say, whilst I agreed with the comment's original context, I don't see any loss of intuition if the renamed enum order became:
> ```
> SK_FixedWidthOnly = 0
> SK_PreferScalable = 1
> SK_PreferFixedWidth = 2
> ```
> and thus you would no longer need these changes.
That's a good point, with the new names of the enum values I agree the change in order is equally intuitive.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101945



More information about the llvm-commits mailing list