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

[Florian Hahn via Phabricator via llvm-commits]

fhahn added inline comments.


================
Comment at: llvm/include/llvm/Transforms/Vectorize/LoopVectorizationLegality.h:99
 
+  enum ScalableForceKind {
+    /// Not selected.
----------------
paulwalker-arm wrote:
> fhahn wrote:
> > paulwalker-arm wrote:
> > > sdesmalen wrote:
> > > > fhahn wrote:
> > > > > can this be an enum class or is the code converting from ints to enum values somewhere?
> > > > There are several conversions between unsigned and the enum, so this requires adding casts to several places.
> > > I prefer that we remain consistent with the other `Kind` enums.
> > Using an `enum class` enforces stricter guarantees and allows the compiler the emit better warnings, which seem like tangible practical benefits. I'm not sure if consistency with other enums outweighs the practical benefits? It's quite common for parts of the codebase to evolve step-by-step.
> > 
> > Using an `enum` until the other enums are migrated leaves the door open for changes that make switching to an `enum class` harder.
> Sure I get that. I'm just saying I prefer all the enums associated with the LoopVectorizeHints class to be handled the same way.
> There are several conversions between unsigned and the enum, so this requires adding casts to several places.

I missed that comment earlier. If there's the need for casting, it's a moot point anyways.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101945