This is an archive of the discontinued LLVM Phabricator instance.

[CodeGen] Move lowerCopy from expandPostRA to TII
ClosedPublic

Authored by yassingh on Jun 6 2023, 5:29 AM.

Details

Summary

This will allow targets to lower their 'copy' instructions easily.

Diff Detail

Unit TestsFailed

Event Timeline

yassingh created this revision.Jun 6 2023, 5:29 AM
Herald added a project: Restricted Project. · View Herald TranscriptJun 6 2023, 5:29 AM
Herald added a subscriber: hiraditya. · View Herald Transcript
yassingh requested review of this revision.Jun 6 2023, 5:29 AM
Herald added a project: Restricted Project. · View Herald TranscriptJun 6 2023, 5:30 AM
cdevadas added inline comments.
llvm/include/llvm/CodeGen/TargetInstrInfo.h
1164

Fix the comment "machine IR" -> "target specific instruction"

arsenm added inline comments.Jun 6 2023, 12:40 PM
llvm/lib/CodeGen/TargetInstrInfo.cpp
721–724

Should pass in TRI for consistency with other TRI functions

728

Don't see the point of the bool return, it just always returns true

yassingh updated this revision to Diff 529239.Jun 7 2023, 3:54 AM

Addressed review comments.

arsenm added inline comments.Jun 7 2023, 4:58 AM
llvm/lib/CodeGen/TargetInstrInfo.cpp
724

Don't need TII, that's just this

yassingh updated this revision to Diff 529361.Jun 7 2023, 10:42 AM

Use 'this' pointer.

yassingh marked 4 inline comments as done.Jun 7 2023, 10:45 AM
arsenm added inline comments.Jun 7 2023, 10:47 AM
llvm/lib/CodeGen/TargetInstrInfo.cpp
746

You don’t need the explicit this

yassingh added inline comments.Jun 7 2023, 11:09 AM
llvm/lib/CodeGen/TargetInstrInfo.cpp
746

Just this one or 'get()' above too?

arsenm added inline comments.Jun 7 2023, 11:24 AM
llvm/lib/CodeGen/TargetInstrInfo.cpp
746

Everywhere

yassingh updated this revision to Diff 529385.Jun 7 2023, 11:29 AM

Removed explicit 'this'

arsenm accepted this revision.Jun 8 2023, 5:34 AM
arsenm added inline comments.
llvm/lib/CodeGen/TargetInstrInfo.cpp
703

Might as well change this to start with lowercase while you're here

This revision is now accepted and ready to land.Jun 8 2023, 5:34 AM
barannikov88 added inline comments.
llvm/include/llvm/CodeGen/TargetInstrInfo.h
1164

Can't it be lowered into several instructions? The comment near copyPhysReg suggests that it can.

arsenm added inline comments.Jun 9 2023, 10:19 AM
llvm/include/llvm/CodeGen/TargetInstrInfo.h
1164

Yes, most copies are expanded to multiple instructions for amdgpu

yassingh updated this revision to Diff 532855.Jun 20 2023, 3:27 AM
yassingh marked 2 inline comments as done.

Rebase.

arsenm accepted this revision.Jun 27 2023, 10:55 AM
This revision was landed with ongoing or failed builds.Jul 3 2023, 8:48 PM
This revision was automatically updated to reflect the committed changes.