-
Notifications
You must be signed in to change notification settings - Fork 15.2k
[LiveIns] Improve recomputeLiveIns() #88951
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Some small changes to recomputeLiveIns() to improve performance: - Instead of copying the list of old live-ins, and then clearing them, a new method swaps the list for an empty one. - getLiveIns() now returns a constant reference to the list As result, the list-data is never copied. Depending on the implementation details of the vector container, it can also save calls to allocate and deallocate memory. I see a small improvement on CTMark with these changes.
| /// Convenience function for recomputing live-in's for a MBB. Returns true if | ||
| /// any changes were made. | ||
| static inline bool recomputeLiveIns(MachineBasicBlock &MBB) { | ||
| inline bool recomputeLiveIns(MachineBasicBlock &MBB) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This needs the extern definition somewhere. Maybe just move the body out of line?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks - the static is there for ages, I forgot to remove it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I mean you need to put it back or ensure this is defined somewhere
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed.
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
|
|
||
| auto newLiveIns = MBB.getLiveIns(); | ||
| return oldLiveIns != newLiveIns; | ||
| auto NewLiveIns = MBB.getLiveIns(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
const & no auto?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changed.
Co-authored-by: Nikita Popov <[email protected]>
Some small changes to recomputeLiveIns() to improve performance:
them, a new method swaps the list for an empty one.
As result, the list-data is never copied. Depending on the implementation
details of the vector container, it can also save calls to allocate
and deallocate memory.
I see a small improvement on CTMark with these changes.