This is an archive of the discontinued LLVM Phabricator instance.

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

Authored by chandlerc on May 24 2017, 10:15 PM.

Details

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.

Diff Detail

Repository
rL LLVM

Event Timeline

chandlerc created this revision.May 24 2017, 10:15 PM

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.

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 accepted this revision.May 24 2017, 10:52 PM

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.

This revision is now accepted and ready to land.May 24 2017, 10:52 PM
davide added inline comments.May 24 2017, 10:53 PM
test/Transforms/GlobalDCE/externally_available.ll
17 ↗(On Diff #100202)

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

chandlerc marked an inline comment as done.May 24 2017, 11:43 PM

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

This revision was automatically updated to reflect the committed changes.