User Details
- User Since
- Dec 13 2019, 12:00 PM (164 w, 3 d)
Jun 25 2020
Jun 24 2020
The change landed: https://github.com/llvm/llvm-project/commit/6a5d7d498c0b16b13ace802f422b223eb510c303
ping
Jun 22 2020
- 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()
Jun 19 2020
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
Jun 18 2020
Jun 17 2020
Addressed to @kbobyrev:
Thank you for the detailed report! I will add new assert/verification code to cover the point you discussed.
Jun 16 2020
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.
Jun 15 2020
- 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
Jun 12 2020
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
Jun 11 2020
Closed due to this commit: https://github.com/llvm/llvm-project/commit/1022b5eb5b37f7dc93ae36002de694541db0e0c1
Jun 10 2020
Ping
Jun 5 2020
Jun 4 2020
I have noticed that MaxEdgeCount is essentially count the same thing as MaxFreq, so I replaced its uses in all the cases.
Jun 3 2020
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
Jun 2 2020
Answering @mtrofin 's comments:
- Refactoring change
May 27 2020
Addressed @davidxl 's comments, did some refactoring
May 4 2020
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
May 1 2020
- 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.
Apr 30 2020
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
Apr 29 2020
Apr 20 2020
- Changed the flag in the test
Apr 13 2020
The wrong diff uploaded previously, reupload
Answering @davidxl 's comments:
- Renamed flags
Apr 10 2020
Transferred the getNumOfCalls functions from the previous review.
Apr 9 2020
LGTM
Apr 8 2020
Apr 7 2020
Apr 6 2020
Apr 1 2020
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.
Mar 31 2020
Mar 30 2020
Excellent!
Can anyone with the commit access make a push of this patch?
Renamed the structure:
CFGDOTInfo -> DOTFuncInfo
Ping
Mar 27 2020
Mar 25 2020
Mar 20 2020
Ping
Mar 6 2020
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.
Feb 28 2020
- 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
Feb 25 2020
- 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:
- The patch is self-contained without this feature. This feature can be added in a follow-up patch.
- The feature itself is not as important and valuable as was first thought of.
Feb 24 2020
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".
Feb 21 2020
Fixed small inaccuracies
As the patch is done, @apilipenko , can you please push it for me?
Sorry, wrong diff