This is an archive of the discontinued LLVM Phabricator instance.

[LTO] Add optimization remarks for removed functions
ClosedPublic

Authored by xazax.hun on Jan 28 2020, 5:21 PM.

Details

Summary

Some functions are not getting added to the combined module that will be optimized later. Knowing which functions were removed could be useful for debugging or for detecting dead code. While getting the list of removed functions is not hard, optimizations remarks were not set up until just before opt was run. I moved this setup process a bit earlier for regular LTO so I can emit remarks. See http://lists.llvm.org/pipermail/llvm-dev/2020-January/138660.html for the discussion.
Note that, I did not change ThinLTO, as those modules will only be available later on.

Diff Detail

Event Timeline

xazax.hun created this revision.Jan 28 2020, 5:21 PM
thegameg added inline comments.
llvm/lib/LTO/LTO.cpp
790

This is not the usual way we add strings with arguments to remarks. Please use << "STR: " << ore::NV("Function", F).

xazax.hun marked an inline comment as done.
  • More idiomatic optimization remark is emitted.
xazax.hun marked an inline comment as done.Jan 29 2020, 10:44 AM
xazax.hun added inline comments.
llvm/lib/LTO/LTO.cpp
790

Thanks!

tejohnson added inline comments.Jan 29 2020, 1:33 PM
llvm/lib/LTO/LTO.cpp
969

Can you add a FIXME here noting that this is checking both Regular and Thin LTO and should be split into two, one checking Regular here and the other checking the ThinLTO index in runThinLTO?

This does work since we always go through this path, but logically it would make more sense to do the ThinLTO checking in runThinLTO. Unfortunately, the regular LTO checking this is doing must be after linkRegularLTO.

llvm/test/LTO/Resolution/X86/dead-strip-fulllto.ll
19

Is the yaml remark more verbose than this (i.e. it includes your message)? Would be good to include more of it.

xazax.hun updated this revision to Diff 241302.Jan 29 2020, 3:02 PM
xazax.hun marked 2 inline comments as done.
  • Addressed more review comments.
xazax.hun marked 2 inline comments as done.Jan 29 2020, 3:04 PM
xazax.hun added inline comments.
llvm/lib/LTO/LTO.cpp
969

Let me know if you prefer to reword this. I am not a native speaker and not familiar with the nomenclature of LLVM.

llvm/test/LTO/Resolution/X86/dead-strip-fulllto.ll
19

Good point, thanks!

tejohnson accepted this revision.Jan 29 2020, 3:48 PM

lgtm

llvm/lib/LTO/LTO.cpp
969

It looks fine, thanks.

This revision is now accepted and ready to land.Jan 29 2020, 3:48 PM
thegameg added inline comments.Jan 29 2020, 4:00 PM
llvm/lib/LTO/LTO.cpp
790

These changes and the test updates look good to me fwiw! Thanks!

This revision was automatically updated to reflect the committed changes.