[PM] Teach the PGO instrumentation pasess to run GlobalDCE before instrumenting code.

[chandlerc]

Summary

This is important in the new pass manager. The old pass manager's
inliner has a small DCE routine embedded within it. The new pass manager
relies on the actual GlobalDCE pass for this.

Without this patch, instrumentation profiling with the new PM results in
massive code bloat in the object files because the instrumentation
itself ends up preventing DCE from working to remove the code.

We should probably change the instrumentation (and/or DCE) so that we
can eliminate dead code even if instrumented, but we shouldn't even
spend the time generating instrumentation for that code so this still
seems like a good patch.

[chandlerc]

Forgot to mention, I also added a test to verify that GlobalDCE won't remove available_externally functions that are actually in use in the module so it won't impair subsequent optimization passes.

[chandlerc]

Add David to the reviewer set in case he can look sooner. I'd love to land this and unblock testing of the new PM + PGO.

[davide]

I can see the bloat as we inline with a ridicolously high threshold before instrumenting without DCE'ing, so the patch seems a decent idea.
Also, DCE is basically free compile-time wise. If you want to improve I'd do it on the DCE side instead of the instrumentation side, but that needs a larger discussion which definitely doesn't fit in the margin of this page.

[davide]

test/Transforms/GlobalDCE/externally_available.ll

17

Minor.
its -> it's
externaly -> externally.

[davidxl]

lgtm

[chandlerc]

Thanks, landing this to fix the immediate issue (with comments addressed).

lib/Passes/PassBuilder.cpp

if (!isOptimizingForSize(Level)) {
 
     // FIXME: Here the old pass manager inserts peephole extensions.
     // Add them when they're supported.
     CGPipeline.addPass(createCGSCCToFunctionPassAdaptor(std::move(FPM)));
 
     MPM.addPass(createModuleToPostOrderCGSCCPassAdaptor(std::move(CGPipeline)));
   }
 
+  // Delete anything that is now dead to make sure that we don't instrument
+  // dead code. Instrumentation can end up keeping dead code around and
+  // dramatically increase code size.
+  MPM.addPass(GlobalDCEPass());
+
   if (RunProfileGen) {
     MPM.addPass(PGOInstrumentationGen());
 
     // Add the profile lowering pass.
     InstrProfOptions Options;
     if (!ProfileGenFile.empty())
       Options.InstrProfileOutput = ProfileGenFile;
     MPM.addPass(InstrProfiling(Options));

test/Transforms/GlobalDCE/externally_available.ll

 ; RUN: opt < %s -globaldce -S | FileCheck %s
 
+; test_global should not be emitted to the .s file.
+; CHECK-NOT: @test_global =
+@test_global = available_externally global i32 4
+
+; test_global2 is a normal global using an available externally function.
+; CHECK: @test_global2 =
+@test_global2 = global i32 ()* @test_function2
+
 ; test_function should not be emitted to the .s file.
-; CHECK-NOT: test_function
+; CHECK-NOT: define {{.*}} @test_function()
 define available_externally i32 @test_function() {
   ret i32 4
 }
 
-; test_global should not be emitted to the .s file.
-; CHECK-NOT: test_global
-@test_global = available_externally global i32 4
-
+; test_function2 isn't actually dead even though its available externaly.
+; CHECK: define available_externally i32 @test_function2()
+define available_externally i32 @test_function2() {
+  ret i32 4
+}