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

[Sander de Smalen via Phabricator via llvm-commits]

sdesmalen added a comment.

Hi @CarolineConcatto thanks for having a look.



================
Comment at: llvm/include/llvm/Transforms/Vectorize/LoopVectorizationLegality.h:132
 
-  bool isScalable() const { return Scalable.Value; }
 
----------------
CarolineConcatto wrote:
> I don't understand where is this function is used, because you've replaced it by isScalableVectorizationPreferred, but you did not changed in any other place. Maybe I am missing something.
> And none of them are used.
I'm not entirely sure what you mean.
`isScalableVectorizationPreferred` is used in `disableScalableVectorization`, which is used in `getWidth` and in LoopVectorize.cpp.


================
Comment at: llvm/include/llvm/Transforms/Vectorize/LoopVectorizationLegality.h:146
+  bool isScalableVectorizationPreferred() const {
+    return Scalable.Value == SK_Preferred;
+  }
----------------
CarolineConcatto wrote:
> What will happened when  Scalable.Value  is SK_Enabled?
> It will not be scalable?
SK_Enabled means that scalable vectorization can be used, but isn't necessarily preferred over fixed-width. In contrast, the purpose of this method is to answer if scalable vectorization is actually preferred.


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorizationLegality.cpp:109
+  // loop hint.
+  if (ScalableVectorization == SK_Disabled)
+    Scalable.Value = SK_Disabled;
----------------
CarolineConcatto wrote:
> Do we needs this?
> Does this line do the same:
> Scalable("vectorize.scalable.enable", ScalableVectorization, HK_SCALABLE),?
This is necessary when both the flag and the attribute are set (through code pragmas). After this change, if the user-code contains pragmas such as `vectorize_width(4, scalable)`, then having `-scalable-vectorization=off` will cause the LV to override the hint so that scalable vectors are not considered.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101945