This is an archive of the discontinued LLVM Phabricator instance.

InlineCost - method ::print() to allow dump of statistics to non-debug builds
ClosedPublic

Authored by knaumov on Jun 19 2020, 9:51 AM.

Details

Summary

This is a logical continuation of https://reviews.llvm.org/D81743.
My intention of the said patch was to introduce a pass that would allow us to print the debug stats to check the functionality of the inline and its decisions outside of debug builds. However, the definition of InlineCostCallAnalyzer::dump() is hidden inside preprocessing macros which caused the fall of several buildbots when I commit the change:
http://lab.llvm.org:8011/builders/fuchsia-x86_64-linux/builds/6688
http://lab.llvm.org:8011/builders/llvm-clang-x86_64-win-fast/builds/17841

The idea of this change is to create another dumping function for InlineCostCallAnalyzer (namely, InlineCostCallAnalyzer::print()) with essentially identical purpose, but with the difference that its definition is not dependent on any preprocessing macros and, consequently build types.

Diff Detail

Event Timeline

knaumov created this revision.Jun 19 2020, 9:51 AM

I believe the reason dump() is hidden behind conditional compilation flags, is to avoid the size overhead in the respective builds.

Alternatively, could your use of dump() be guarded by similar conditional compilation flags, and, then, conditionally-depend on that in tests that would use the feature?

@mtrofin, I think it makes sense to have these tests available in all configurations. We have a bunch of printer passes which are not conditioned by NDEBUG.

llvm/lib/Analysis/InlineCost.cpp
2195–2196

Please, avoid duplication and call print here.

Answering @mtrofin :
Although what you suggested is a valid solution, it will go against the point of this whole pass - to enable the checks of inliner's inner works for all builds, not just the debug ones. If we apply your solution, then still we will get several builds which will avoid being checked due to the conditioning flags.

Answering @mtrofin :
Although what you suggested is a valid solution, it will go against the point of this whole pass - to enable the checks of inliner's inner works for all builds, not just the debug ones. If we apply your solution, then still we will get several builds which will avoid being checked due to the conditioning flags.

Ah - and we don't seem to have non-debug build bots with LLVM_ENABLE_DUMP enabled.

knaumov updated this revision to Diff 272500.Jun 22 2020, 10:53 AM
  • To avoid redundancy, used the preprocessing function DEBUG_PRINT_STAT defined for ::dump()
mtrofin added inline comments.Jun 22 2020, 11:22 AM
llvm/lib/Analysis/InlineCost.cpp
2175

maybe I'm missing something, but why not just call print() from dump() ?

knaumov updated this revision to Diff 272510.Jun 22 2020, 11:49 AM
  • Because print() performs the same functionality as did dump(), instead of copying the code just call print() from dump()
knaumov marked 2 inline comments as done.Jun 24 2020, 11:39 AM

ping

This revision is now accepted and ready to land.Jun 24 2020, 12:02 PM
This revision was automatically updated to reflect the committed changes.