[Pipelines] Move LTO SCCP and BDCE out of addVectorPasses()

[nikic]

Summary

I'm trying to reconcile the differences in addVectorPasses() between LTO and ThinLTO/default. These two passes are only present in the LTO version, while for ThinLTO/default these passes run earlier (part of simplification, not optimization). The exact position these run in ThinLTO/default doesn't exist in the LTO pipeline, so I've put them in approximately the same place.

After this change the ThinLTO/default version of addVectorPasses() only runs more passes or in a different order.

[aeubanks]

lgtm

[dmgreen]

Hello. This sounds OK, but apparently it can be pretty beneficial to run BDCE after the second round of unrolling. Unfortunately this may be wrapped up with some downstream compiler changes with a downstream test, it would make it hard to produce a reproducer. It is apparently after the second round of unrolling in this particular test. Otherwise I don't see any issues past that one case.

[nikic]

In D149448#4309461, @dmgreen wrote:

Hello. This sounds OK, but apparently it can be pretty beneficial to run BDCE after the second round of unrolling. Unfortunately this may be wrapped up with some downstream compiler changes with a downstream test, it would make it hard to produce a reproducer. It is apparently after the second round of unrolling in this particular test. Otherwise I don't see any issues past that one case.

Even without a full reproducer, would it be possible to share the IR diff for the BDCE pass for that particular test? Just to get an idea for what makes it beneficial to run it after runtime unrolling. (Note that I've currently scheduled it before full unrolling as well, but based on your description it sounds like moving it after full unrolling wouldn't help either.)

[dmgreen]

Even without a full reproducer, would it be possible to share the IR diff for the BDCE pass for that particular test? Just to get an idea for what makes it beneficial to run it after runtime unrolling. (Note that I've currently scheduled it before full unrolling as well, but based on your description it sounds like moving it after full unrolling wouldn't help either.)

Yeah, it appears to need to run after the unrolling in vectorPasses (or after vectorPasses). After full unrolling didn't seem to help.

I wouldn't want to block things just for this case. There is chance it is just a bad benchmark that doesn't store any useful results. It looks like it removed interconnected phi nodes that have no other uses. I will try to send you what I can get, but like I said I only saw it come up in this one case.

llvm/lib/Passes/PassBuilderPipelines.cpp

void PassBuilder::addVectorPasses(OptimizationLevel Level,
   FPM.addPass(SimplifyCFGPass(SimplifyCFGOptions()
                                   .forwardSwitchCondToPhi(true)
                                   .convertSwitchRangeToICmp(true)
                                   .convertSwitchToLookupTable(true)
                                   .needCanonicalLoops(false)
                                   .hoistCommonInsts(true)
                                   .sinkCommonInsts(true)));
 
-  if (IsFullLTO) {
-    FPM.addPass(SCCPPass());
+  if (IsFullLTO)
     FPM.addPass(InstCombinePass());
-    FPM.addPass(BDCEPass());
-  }
 
   // Optimize parallel scalar instruction chains into SIMD instructions.
   if (PTO.SLPVectorization) {
     FPM.addPass(SLPVectorizerPass());
     if (Level.getSpeedupLevel() > 1 && ExtraVectorizerPasses) {
       FPM.addPass(EarlyCSEPass());
     }
   }
PassBuilder::buildLTODefaultPipeline(OptimizationLevel Level,
   // Remove dead memcpy()'s.
   MainFPM.addPass(MemCpyOptPass());
 
   // Nuke dead stores.
   MainFPM.addPass(DSEPass());
   MainFPM.addPass(MoveAutoInitPass());
   MainFPM.addPass(MergedLoadStoreMotionPass());
 
+  MainFPM.addPass(SCCPPass());
+  MainFPM.addPass(BDCEPass());
+
   LoopPassManager LPM;
   if (EnableLoopFlatten && Level.getSpeedupLevel() > 1)
     LPM.addPass(LoopFlattenPass());
   LPM.addPass(IndVarSimplifyPass());
   LPM.addPass(LoopDeletionPass());
   // FIXME: Add loop interchange.
 
   // Unroll small loops and perform peeling.

llvm/test/Other/new-pm-lto-defaults.ll

 ; CHECK-O23SZ-NEXT: Running pass: LICMPass on loop
 ; CHECK-O23SZ-NEXT: Running pass: GVNPass on foo
 ; CHECK-O23SZ-NEXT: Running analysis: MemoryDependenceAnalysis on foo
 ; CHECK-O23SZ-NEXT: Running pass: MemCpyOptPass on foo
 ; CHECK-O23SZ-NEXT: Running pass: DSEPass on foo
 ; CHECK-O23SZ-NEXT: Running analysis: PostDominatorTreeAnalysis on foo
 ; CHECK-O23SZ-NEXT: Running pass: MoveAutoInitPass on foo
 ; CHECK-O23SZ-NEXT: Running pass: MergedLoadStoreMotionPass on foo
+; CHECK-O23SZ-NEXT: Running pass: SCCPPass on foo
+; CHECK-O23SZ-NEXT: Running pass: BDCEPass on foo
+; CHECK-O23SZ-NEXT: Running analysis: DemandedBitsAnalysis on foo
 ; CHECK-O23SZ-NEXT: Running pass: LoopSimplifyPass on foo
 ; CHECK-O23SZ-NEXT: Running pass: LCSSAPass on foo
 ; CHECK-O23SZ-NEXT: Running pass: IndVarSimplifyPass on loop
 ; CHECK-O23SZ-NEXT: Running pass: LoopDeletionPass on loop
 ; CHECK-O23SZ-NEXT: Running pass: LoopFullUnrollPass on loop
 ; CHECK-O23SZ-NEXT: Running pass: LoopDistributePass on foo
 ; CHECK-O23SZ-NEXT: Running analysis: LoopAccessAnalysis on foo
 ; CHECK-O23SZ-NEXT: Running pass: LoopVectorizePass on foo
-; CHECK-O23SZ-NEXT: Running analysis: DemandedBitsAnalysis on foo
 ; CHECK-O23SZ-NEXT: Running pass: LoopUnrollPass on foo
 ; CHECK-O23SZ-NEXT: WarnMissedTransformationsPass on foo
 ; CHECK-O23SZ-NEXT: Running pass: SROAPass on foo
 ; CHECK-O23SZ-NEXT: Running pass: InstCombinePass on foo
 ; CHECK-O23SZ-NEXT: Running pass: SimplifyCFGPass on foo
-; CHECK-O23SZ-NEXT: Running pass: SCCPPass on foo
 ; CHECK-O23SZ-NEXT: Running pass: InstCombinePass on foo
-; CHECK-O23SZ-NEXT: Running pass: BDCEPass on foo
 ; CHECK-O2-NEXT: Running pass: SLPVectorizerPass on foo
 ; CHECK-O3-NEXT: Running pass: SLPVectorizerPass on foo
 ; CHECK-OS-NEXT: Running pass: SLPVectorizerPass on foo
 ; CHECK-O23SZ-NEXT: Running pass: VectorCombinePass on foo
 ; CHECK-O23SZ-NEXT: Running pass: AlignmentFromAssumptionsPass on foo
 ; CHECK-O23SZ-NEXT: Running pass: InstCombinePass on foo
 ; CHECK-EP-Peephole-NEXT: Running pass: NoOpFunctionPass on foo
 ; CHECK-O23SZ-NEXT: Running pass: JumpThreadingPass on foo