This is an archive of the discontinued LLVM Phabricator instance.

[FastISel] Remove kill tracking
ClosedPublic

Authored by nikic on Mar 9 2021, 2:42 PM.

Details

Summary

This is a followup to D98145: As far as I know, tracking of kill flag in FastISel is just a compile-time optimization. However, I'm not actually seeing any compile-time regression when removing the tracking (http://llvm-compile-time-tracker.com/compare.php?from=f111dc7cfcda28597b6f26728f7e0cc584e48460&to=f51b35fba39dad43fd22cdbdbea3112a3401c06b&stat=instructions -- the tramp3d-v4 improvement is noise). I think that might have been more important in the past, before FastRA was switched to allocate instructions in reverse order, which means that it discovers kills as a matter of course.

As such, the kill tracking doesn't really seem to have a purpose, and just adds additional complexity and potential for errors. This patch removes it entirely. The primary changes are dropping the hasTrivialKill() method and removing the kill arguments from the emitFast methods. The rest is mechanical fixup.

Diff Detail

Event Timeline

nikic created this revision.Mar 9 2021, 2:42 PM
nikic requested review of this revision.Mar 9 2021, 2:42 PM
Herald added a project: Restricted Project. · View Herald TranscriptMar 9 2021, 2:42 PM

I've no objections to this - anyone else?

nikic added a comment.Mar 20 2021, 2:18 AM

Any more thoughts on this?

Please can you add an entry to the release notes - this might affect downstream targets so we should document this.

Reviewers - any comments?

@nikic OK I think this just needs a release notes entry and its ready to go

nikic updated this revision to Diff 335069.Apr 3 2021, 2:07 AM

Add release note.

RKSimon accepted this revision.Apr 3 2021, 3:12 AM

LGTM

This revision is now accepted and ready to land.Apr 3 2021, 3:12 AM
This revision was landed with ongoing or failed builds.Apr 3 2021, 6:50 AM
This revision was automatically updated to reflect the committed changes.