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

[Caroline via Phabricator via llvm-commits]

CarolineConcatto added a comment.

Hey @sdesmalen 
I have more questions about the implementation.



================
Comment at: llvm/include/llvm/Transforms/Vectorize/LoopVectorizationLegality.h:147
+  /// the cost-model is inconclusive.
+  bool isScalableVectorizationPreferred() const {
+    return Scalable.Value == SK_Preferred;
----------------
This function is not used.


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorizationLegality.cpp:86
+  case HK_SCALABLE:
+    return (Val == SK_Disabled || Val == SK_Preferred);
   }
----------------
Should it be all the possible solutions here:
Val == SK_Disabled
Val == SK_Preferred
Val == SK_Enabled
Val == SK_Undefined



================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorizationLegality.cpp:99
       Predicate("vectorize.predicate.enable", FK_Undefined, HK_PREDICATE),
-      Scalable("vectorize.scalable.enable", false, HK_SCALABLE), TheLoop(L),
-      ORE(ORE) {
+      Scalable("vectorize.scalable.enable", SK_Undefined, HK_SCALABLE),
+      TheLoop(L), ORE(ORE) {
----------------
Can vectorize.scalable.enable have the same values as scalable-vectorization?
Which values should  LoopVectorizeHints::Hint::validate check?


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorizationLegality.cpp:117
+    Scalable.Value = SK_Disabled;
+
   if (IsVectorized.Value != 1)
----------------
Just to double check:
 if **Scalable.Value != SK_Undefined**  and ** ScalableVectorization != SK_Disabled** 
 **Scalable.value** will have the value set in **vectorize.scalable.enable**, is that correct?
 and not in 
**scalable-vectorization**


================
Comment at: llvm/test/Transforms/LoopVectorize/AArch64/scalable-vf-analysis.ll:14
+; CHECK_SCALABLE_PREFERRED: LV: Found feasible scalable VF = vscale x 4
+; CHECK_SCALABLE_DISABLED-NOT: LV: Found feasible scalable VF
 ; CHECK_SCALABLE_ON_MAXBW: LV: Found feasible scalable VF = vscale x 16
----------------
Is there a way to tell that VF scalable will not be used?
Only this:
**LV: Found feasible scalable VF**
is quite odd, because it does not explicitly says it will not use scalable.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101945