LLVM: Expose SimplifyCFGPass (as used in PassBuilder::buildModuleOptimizationPipeline) in PassRegistry.def

[georgevdd]

Summary

This makes it possible to write a pipeline string for opt that does the same thing as the hard-coded optimization pipeline. c.f. early-cse and early-cse-memssa.

[georgevdd]

Is there anything I need to do to help this along? It's over a month old, now.

[spatel]

In D52227#1272506, @georgevdd wrote:

Is there anything I need to do to help this along? It's over a month old, now.

Feel free to ping weekly to push this faster. I lost track of the patch.

The backstory for those pass options is that we actually had a named pass ("latesimplifycfg") encapsulating them before we decided we wanted the flexibility to be able to turn things on/off with finer granularity:
https://reviews.llvm.org/D38631

...so this patch is proposing to partially restore that (it doesn't change anything for the default pass manager?).

It's going to effectively make the bundle of options shown here a standard config by giving that bundle a name.

There was a suggestion to break SimplifyCFG into multiple passes rather than providing a pile of options here:
https://reviews.llvm.org/D38566

That would be ideal because it would give us both the flexibility and the test-ability that this patch is seeking?
I don't know how much work that would be, so I'm not sure if it's fair to ask for that change.
If we decide that this patch isn't the wrong direction, it will need tests to verify the new functionality.

[georgevdd]

1. Some backstory from my end:
   
   I am interested in exploring the phase ordering problem. So my work is dependent on being able to construct pipelines using their regular components, but in a different order from what (say) default<O3> would produce. This is almost doable using PassBuilder, but not if its hard-wired subpipeline functions use constructor parameters that are not represented in PassRegistry.def.
   
   Meanwhile, @chandlerc expressed some intention for the new PassManager framework to be able to print diagnostics representing the structure of a pipeline, the same way that the old PassManager could (with -debug-pass, IIRC). I have some not-quite-finished work to achieve that, but it too depends on the DSL that PassBuilder accepts being able to represent the passes that actually occur in the pipelines that it builds.

2. With regard to your specific remarks:
   • Yes, this patch intends not to change the behavior of any existing pass pipelines, including specifically the default pass manager;
   • Yes, it's simply trying to give a usable name to the bundle of options that occurs in practice in the default optimization pipeline;
   • Yes, breaking SimplifyCFG into smaller named parts as suggested in the discussion to which you refer would also solve the problem, but I share your ignorance about how difficult that would be;
   • I am keen to supply appropriate tests but, as a newcomer, probably a poor judge of what appropriate looks like.

More generally, does it seem to you reasonable to aim to support the ability to represent exactly the default pipelines using the names of their elements? Or is the overall direction of development away from that?

[spatel]

In D52227#1273643, @georgevdd wrote:

• I am keen to supply appropriate tests but, as a newcomer, probably a poor judge of what appropriate looks like.

I don't have much experience with the pass manager level, but you probably want to have an IR test to confirm that the options set by "simplify-cfg-opt" are actually enabled by checking that the IR is transformed correctly. See the previously mentioned patches for possible examples.

More generally, does it seem to you reasonable to aim to support the ability to represent exactly the default pipelines using the names of their elements? Or is the overall direction of development away from that?

I understand the goal, but I'm not sure how to best achieve it. If you've already discussed this idea with others, then it should be fine. I'll ask others to continue the review of this patch because I don't have more to say here.

[john.brawn]

Having some way to generate a pipeline string which matches what opt does by default is definitely a good thing. I think that long-term splitting up simplifycfg is probably the best way, and I think I'm going to have to do some of that myself as part of D50723 (as the current implementation causes a lot of problems for bugpoint as the pass list it deduces doesn't do the same thing as -Owhatever), but doing it in this way for now seems fine.

include/llvm/Transforms/Scalar/SimplifyCFG.h

 class SimplifyCFGPass : public PassInfoMixin<SimplifyCFGPass> {
   SimplifyCFGOptions Options;
 
 public:
   /// The default constructor sets the pass options to create canonical IR,
   /// rather than optimal IR. That is, by default we bypass transformations that
   /// are likely to improve performance but make analysis for other passes more
   /// difficult.
-  SimplifyCFGPass()
-      : SimplifyCFGPass(SimplifyCFGOptions()
-                            .forwardSwitchCondToPhi(false)
-                            .convertSwitchToLookupTable(false)
-                            .needCanonicalLoops(true)
-                            .sinkCommonInsts(false)) {}
+  SimplifyCFGPass() : SimplifyCFGPass(false) {}
 
+  explicit SimplifyCFGPass(bool optimizing) : SimplifyCFGPass(
+    SimplifyCFGOptions()
+        .forwardSwitchCondToPhi(optimizing)
+        .convertSwitchToLookupTable(optimizing)
+        .needCanonicalLoops(!optimizing)
+        .sinkCommonInsts(optimizing)) {}
 
   /// Construct a pass with optional optimizations.
   SimplifyCFGPass(const SimplifyCFGOptions &PassOptions);
 
   /// Run the pass over the function.
   PreservedAnalyses run(Function &F, FunctionAnalysisManager &AM);
 };
 
 }
 
 #endif

lib/Passes/PassBuilder.cpp

PassBuilder::buildModuleOptimizationPipeline(OptimizationLevel Level,
   // optimizations. These are run afterward as they might block doing complex
   // analyses and transforms such as what are needed for loop vectorization.
 
   // Cleanup after loop vectorization, etc. Simplification passes like CVP and
   // GVN, loop transforms, and others have already run, so it's now better to
   // convert to more optimized IR using more aggressive simplify CFG options.
   // The extra sinking transform can create larger basic blocks, so do this
   // before SLP vectorization.
-  OptimizePM.addPass(SimplifyCFGPass(SimplifyCFGOptions().
-                                     forwardSwitchCondToPhi(true).
-                                     convertSwitchToLookupTable(true).
-                                     needCanonicalLoops(false).
-                                     sinkCommonInsts(true)));
+  OptimizePM.addPass(SimplifyCFGPass(/*optimizing=*/true));
 
   // Optimize parallel scalar instruction chains into SIMD instructions.
   OptimizePM.addPass(SLPVectorizerPass());
 
   OptimizePM.addPass(InstCombinePass());
 
   // Unroll small loops to hide loop backedge latency and saturate any parallel
   // execution resources of an out-of-order processor. We also then need to

lib/Passes/PassRegistry.def

 FUNCTION_PASS("print<loops>", LoopPrinterPass(dbgs()))
 FUNCTION_PASS("print<memoryssa>", MemorySSAPrinterPass(dbgs()))
 FUNCTION_PASS("print<phi-values>", PhiValuesPrinterPass(dbgs()))
 FUNCTION_PASS("print<regions>", RegionInfoPrinterPass(dbgs()))
 FUNCTION_PASS("print<scalar-evolution>", ScalarEvolutionPrinterPass(dbgs()))
 FUNCTION_PASS("reassociate", ReassociatePass())
 FUNCTION_PASS("sccp", SCCPPass())
 FUNCTION_PASS("simplify-cfg", SimplifyCFGPass())
+FUNCTION_PASS("simplify-cfg-opt", SimplifyCFGPass(true))
 FUNCTION_PASS("sink", SinkingPass())
 FUNCTION_PASS("slp-vectorizer", SLPVectorizerPass())
 FUNCTION_PASS("speculative-execution", SpeculativeExecutionPass())
 FUNCTION_PASS("spec-phis", SpeculateAroundPHIsPass())
 FUNCTION_PASS("sroa", SROA())
 FUNCTION_PASS("tailcallelim", TailCallElimPass())
 FUNCTION_PASS("unreachableblockelim", UnreachableBlockElimPass())
 FUNCTION_PASS("unroll", LoopUnrollPass())