This is an archive of the discontinued LLVM Phabricator instance.

[BOLT] Fix sorting functions by execution count
ClosedPublic

Authored by spupyrev on Jun 14 2023, 1:59 PM.

Details

Summary

I noticed that -reorder-functions=exec-count doesn't work as expected due to
a bug in the comparison function (which isn't symmetric). It is questionable
whether anyone would want to ever use the sorting method (as sorting by say
density is much better in all cases) but it is probably better to fix the bug.

Diff Detail

Event Timeline

spupyrev created this revision.Jun 14 2023, 1:59 PM
Herald added a reviewer: Amir. · View Herald Transcript
Herald added a reviewer: maksfb. · View Herald Transcript
Herald added a project: Restricted Project. · View Herald Transcript
spupyrev edited the summary of this revision. (Show Details)Jun 14 2023, 2:02 PM
spupyrev published this revision for review.Jun 14 2023, 2:08 PM
Herald added a project: Restricted Project. · View Herald TranscriptJun 14 2023, 2:08 PM

Can we have a small test case showing the bug?

spupyrev updated this revision to Diff 537826.EditedJul 6 2023, 12:02 PM

Added a test. Without the change, the output is

...
BOLT-INFO: Starting pass: reorder-functions
BOLT-INFO: hot func main (400)
BOLT-INFO: hot func func1 (500)
BOLT-INFO: hot func func2 (1500)
BOLT-INFO: hot func func3 (100)
BOLT-INFO: hot func func4 (99)
BOLT-INFO: hot func func5 (110)
...
Amir accepted this revision.Aug 1 2023, 7:47 PM
This revision is now accepted and ready to land.Aug 1 2023, 7:47 PM
spupyrev updated this revision to Diff 550891.Aug 16 2023, 2:20 PM

rebase & adjust logging

This revision was automatically updated to reflect the committed changes.