The change landed: https://github.com/llvm/llvm-project/commit/6a5d7d498c0b16b13ace802f422b223eb510c303

ping

• Because print() performs the same functionality as did dump(), instead of copying the code just call print() from dump()

• To avoid redundancy, used the preprocessing function DEBUG_PRINT_STAT defined for ::dump()

All of the preparational patches were approved, but there was an issue with integration which is being resolved in D82205

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.

Closed due to commit: https://github.com/llvm/llvm-project/commit/41d53194fb9d5aba3e4861233b1af9cb62cc999a

Addressed to @kbobyrev:
Thank you for the detailed report! I will add new assert/verification code to cover the point you discussed.

ping

All of the related patches were successfully reviewed and landed!
Thanks to everyone who has participated in this long journey!

This change was implemented as a part of Heat Coloring (1/3): Adding distinct structure for CFGDOTInfo (D76820). Closed by commit https://reviews.llvm.org/rG3f995ce8b54ca6094fd47a5f1090ef6ce367ded2

• Changed the comments to better reflect the usage of the pass
• Changed the summary of the differential

Sorry, submitted wrong diff

• Refactored the comment

For now, my view of the pass is to be able to tell what optimizations are seen and predicted by InlineCost. E.g. is a follow-up patch D81024 which enables printing of constants to which the instruction will be simplified. For now, if you've written a simplification for InlineCost that constant-folds some sequence of instructions, you have no way of writing test for it except by setting the inline threshold so low, so the fact of simplification will inline the method. This pass provides an easy way to see such cases directly. Also, the fact of simplification itself is independent of InlineParams, so the pass would work as expected.

• Added flag for the change which is true by default

I have been struggling to collect the data @mtrofin has asked for to prove the usefulness of the patch. I will continue to do so, but meanwhile, I suggest accepting the change. Once I have gathered needed data, I will post new differential presenting the results and (most likely) deleting the flag.

ping

Changed the creation of InlineParams back to default params and added a "to-do" comment.
For the purposes of this pass, the idea of which is to give access for non-debug builds to tests that use debug functionality, the InlineParams can be taken as default. My suggestion is to leave the extension of the pass to parametrization with different InlineParams as a "to-do" for now.

A question to @mtrofin:
I have changed the creation of InlineParams from

const InlineParams Params = llvm::getInlineParams();

which invokes the creation of InlineParams with DefaultThreshold to

Closed due to this commit: https://github.com/llvm/llvm-project/commit/1022b5eb5b37f7dc93ae36002de694541db0e0c1

Ping

I have noticed that MaxEdgeCount is essentially count the same thing as MaxFreq, so I replaced its uses in all the cases.

Addressed @mtrofin 's comments:

• • Before the change, we had CostAnnotationWriter that was used just to display the cost difference per instruction, hence the CostThresholdMap (which now became InstructionCostDetailMap) was in the printer itself. Now, the change changed that by putting the map into InlineCostCallAnalyzer to keep all the data in this class.
• The move of code was unnecessary, changed it

Addressed @davidxl 's comments:

• Added comments
• Brought for-loop iteration over functions in the module into the previous one

Answering @mtrofin 's comments:

• Refactoring change

Addressed @davidxl 's comments, did some refactoring

This Callgraph was generated from test_heat.ll
You can see, as foo is not being called by anyone, it is marked as the coldest block. Bar functions are ranked from the least number to the greatest by their hotness which makes sense - whenever function bar1 is called, it is calling all of the other bars that follow and so on. Thus, bar5 is the hottest function in the .ll file

• Fixed the functionality of heat coloring - now the coloring of CallGraph is done by analyzing how many times the given function was called. The more times it's being called - the hotter ("redder") it will be on the graph.

This revert was necessary as the test used "-debug-only" option without

; Require asserts for -debug-only
; REQUIRES: asserts

Closing due to commit: https://github.com/llvm/llvm-project/commit/055f58fcfc618cdad0a8cb66fbd4657cdc0d05e9

• Changed the flag in the test

The wrong diff uploaded previously, reupload

Answering @davidxl 's comments:

• Renamed flags

Transferred the getNumOfCalls functions from the previous review.

LGTM

Answering @davidxl 's comments:

• Fixed the introduction of getNumOfCalls methods.
• Concerning the color-blind accessibility - from my research, I have found that the combination of blue and red and all of the intermediate colors are well perceived by the majority of types of color-blind people. I have not found a way to make edges of the boxes gradually bolder to reflect the gradation from cold to hot methods. If any such solutions were to be found, it would be great to implement them, but my suggestion is to do it as a separate future patch while the current series of patches introduces only necessary functionality.
• Clang-Format: I have applied the clang-format on the said file and found out that not only my changes were formatted, but most of the original file as well. I have reverted the clang-format, as it makes the diff big and hard to follow. Of course, once I am going to be pushing the changes, I will apply clang-format over all the files that were altered by my patches.

Excellent!
Can anyone with the commit access make a push of this patch?

Renamed the structure:
CFGDOTInfo -> DOTFuncInfo

Ping

Ping

Fixes:

• Added documentation to the new functions
• Added two test cases - one for multigraph, the second one for heat CallGraph/CFG specifically
• Reduced the quadratic behavior that was mentioned by @davidxl . There is no way to reduce the behavior to smaller than O(n^2), but I've made it to (n^2)/2. There is no need to go beyond this as this is just a debugging feature that is not to be used outside of debugging.

• Restoring the effect of clang-format

The effect of clang-format was restored for better readability of the patch

• Deleted Heuristic fully

As I said in the previous diffs, Heuristic bool was always true and never changing.

• Transferred CFGPrinter back to Function Pass

The patch was made module in order to implement the comparison between BB across functions. As this patch is not targeting this behavior, the pass was decided to remain Function Pass

• Small stylistics changes

• Removed flag that was targeting coloring BB based on their relative hotness across the module.

I have decided to do that for the following reasons:

1. The patch is self-contained without this feature. This feature can be added in a follow-up patch.
2. The feature itself is not as important and valuable as was first thought of.

Changes:

• Renamed a flag ("callgraph-full" -> "call-multigraph") to reflect its purpose more clearly
• Removed unnecessary if-check
• General refactoring and restructuring
• Removed useHeuristic from the code - from the structure of the code it is vivid that useHeuristic variable used in getMaxFreq functions is always set to "true" and never becomes "false".

Fixed small inaccuracies
As the patch is done, @apilipenko , can you please push it for me?

Sorry, wrong diff