[PATCH] D115651: [LV] Enable scalable vectorization by default for SVE cores.

[Paul Walker via Phabricator via llvm-commits]

paulwalker-arm accepted this revision.
paulwalker-arm added a comment.
This revision is now accepted and ready to land.

A few things to consider plus a bug in `ForceScalableVectorization`'s help text but otherwise this looks good to me.



================
Comment at: llvm/include/llvm/Transforms/Vectorize/LoopVectorizationLegality.h:108
     /// default when the scalable.enable hint is enabled through a pragma.
     SK_PreferScalable = 1,
   };
----------------
rogue comma


================
Comment at: llvm/include/llvm/Transforms/Vectorize/LoopVectorizationLegality.h:125
   ElementCount getWidth() const {
-    return ElementCount::get(Width.Value,
-                             isScalableVectorizationExplicitlyEnabled());
+    return ElementCount::get(Width.Value, Scalable.Value == 1);
   }
----------------
I guess by this point you're saying `Scalable` is now effectively a bool so perhaps just `(Width.Value, Scalable.Value)`?

Either way, we shouldn't use magic values so either treat it as a bool or use the enum values.


================
Comment at: llvm/include/llvm/Transforms/Vectorize/LoopVectorizationLegality.h:147
   /// \return true if scalable vectorization has been explicitly disabled.
-  bool isScalableVectorizationDisabled() const {
-    return Scalable.Value == SK_FixedWidthOnly;
-  }
+  bool isScalableVectorizationDisabled() const { return Scalable.Value == 0; }
 
----------------
As above perhaps `!Scalable.Value`?


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorizationLegality.cpp:69-70
+                LoopVectorizeHints::SK_PreferScalable, "on",
+                "Scalable vectorization is available, but favor fixed-width "
+                "vectorization when the cost is inconclusive.")));
 
----------------
You've kept the wrong help text.


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorizationLegality.cpp:135
+
+  // No scalable vectorization is the default if no preference is specified.
   if ((LoopVectorizeHints::ScalableForceKind)Scalable.Value == SK_Unspecified)
----------------
"Scalable vectorization is disabled if no preference is specified"?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115651