[PATCH] D115052: [Passes] Only run extra vector passes if loops have been vectorized.

[Arthur Eubanks via Phabricator via llvm-commits]

aeubanks added a comment.

this is a little different from D113947 <https://reviews.llvm.org/D113947> in that we're using an analysis to keep track of something that must happen if a pass did something, whereas D113947 <https://reviews.llvm.org/D113947> uses an analysis to keep track of if we *don't* have to do something
the only other thing I can think of that's somewhat similar is the devirtualization wrapper where we keep track of the number of direct/indirect calls, and also using value handles to keep track of if any devirtualization happened, and rerun passes if it did

I guess there isn't a great alternative to something like this unless we have some sort of marker within the IR itself, which doesn't really make any sense
so something along these lines seems fine to me



================
Comment at: llvm/include/llvm/Transforms/Vectorize/LoopVectorize.h:98
+/// extra simplifications could be beneficial).
+class ExtraVectorPassManager : public FunctionPassManager {
+  /// Set of passes to run conditionally.
----------------
fhahn wrote:
> aeubanks wrote:
> > can `ExtraVectorPassManager` only contain `ConditionalPasses` and we have a separate FPM for the normal passes? separation of concerns, seems like this is doing two things that could be separated
> The reason why `ExtraVectorPassManager` contains both the conditional and unconditional passes is that this way we can check whether `ShouldRunExtraVectorPasses` is available *before* the unconditional passes run.
> 
> If we would run the unconditional passes outside of `ExtraVectorPassManager`, then any change they make would invalidate `ShouldRunExtraVectorPasses` IIUC. Unless we teach all those passes to preserve it. Or perhaps there's a different alternative?
you could override the `invalidate()` method of `Result` to be stateless, e.g. following
```
bool GlobalsAAResult::invalidate(Module &, const PreservedAnalyses &PA,
                                 ModuleAnalysisManager::Invalidator &) {
  // Check whether the analysis has been explicitly invalidated. Otherwise, it's
  // stateless and remains preserved.
  auto PAC = PA.getChecker<GlobalsAA>();
  return !PAC.preservedWhenStateless();
}
```

this makes it so it's only invalidated if it's specifically abandoned

this should make it so this only needs to wrap the `ConditionaPasses` and not be its own `FunctionPassManager`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115052