[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