[PATCH] D81345: [LV] Vectorize without versioning-for-unit-stride under -Os/-Oz

[Florian Hahn via Phabricator via llvm-commits]

fhahn added inline comments.


================
Comment at: llvm/lib/Analysis/LoopAccessAnalysis.cpp:1821
 
+  const bool EnableMemAccessVersioningOfLoop =
+      EnableMemAccessVersioning &&
----------------
Ayal wrote:
> fhahn wrote:
> > I think it might be slightly preferable to let LV drive the decision whether to version or not based on cost estimates (and LAA is used by other passes as well, which might have different requirements).
> > 
> > Did you consider disabling generating the codes for  (I think it should happen `emitSCEVChecks`) conditionally on optsize? IIUC we should only need to generate code for predicates, if either we need runtime checks or have symbolic strides. And runtime checks are already rejected in `runtimeChecksRequired`.
> Would it be better if LV passes a "only one copy of the loop is allowed" flag to analyzeLoop(), via the constructor of LAI, instead of the latter checking for OptSize? This requirement of OptSize is common to all other passes, right?
> 
> Suppressing emitSCEVChecks would appease the assert, but LAI should refrain from collecting Strided Accesses in order (for getPtrStride()) to consider related accesses as non-unit strided accesses. It already does so under a cl::opt flag, for all its users.
> Would it be better if LV passes a "only one copy of the loop is allowed" flag to analyzeLoop(), via the constructor of LAI, instead of the latter checking for OptSize? This requirement of OptSize is common to all other passes, right?

(Sorry for the long delay!)

I think something like that would be preferable, as it makes explicit the interaction between LV/LAI. But I am not sure if that is possible at the moment though, because we get LAI as analysis. 

I am not sure if there's a feasible alternative without too much refactoring and it is probably not worth blocking the change on that, especially given that there's already precedence for using opt flags in a similar way.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81345