This is an archive of the discontinued LLVM Phabricator instance.

[AMDGPU] gfx11 export instructions
ClosedPublic

Authored by Joe_Nash on May 17 2022, 11:58 AM.

Details

Reviewers
foad
dp
arsenm
Group Reviewers
Restricted Project
Commits
rG1a51ab766f47: [AMDGPU] gfx11 export instructions
Summary

Contributors:
Jay Foad <jay.foad@amd.com>
Dmitry Preobrazhensky <d-pre@mail.ru>

Patch 10/N for upstreaming of AMDGPU gfx11 architecture.

Depends on D125822

Diff Detail

Event Timeline

Joe_Nash created this revision.May 17 2022, 11:58 AM
Herald added a project: Restricted Project. · View Herald TranscriptMay 17 2022, 11:58 AM
Joe_Nash requested review of this revision.May 17 2022, 11:58 AM
Herald added a project: Restricted Project. · View Herald TranscriptMay 17 2022, 11:58 AM
Joe_Nash added reviewers: foad, dp, arsenm, Restricted Project.May 17 2022, 12:01 PM
arsenm added inline comments.May 17 2022, 1:58 PM
llvm/lib/Target/AMDGPU/Disassembler/AMDGPUDisassembler.cpp
643–644

Why not remove them then? I guess that would just move the handling from MI->MC from the pseudo

foad added inline comments.May 18 2022, 1:48 AM
llvm/lib/Target/AMDGPU/Disassembler/AMDGPUDisassembler.cpp
643–644

Probably just because I didn't know how to implement that. Is there a good place to do stuff like this when lowering from MI to MC?

Joe_Nash added inline comments.May 19 2022, 10:08 AM
llvm/lib/Target/AMDGPU/Disassembler/AMDGPUDisassembler.cpp
643–644

It seems like there are 3 options: different pseudos between subtargets, handle this in MI->MC (I don't know if there are any examples of this?), and in the asm/disasm (quite common in AMDGPU). So is there anything to do here?

dp added inline comments.May 25 2022, 5:18 AM
llvm/lib/Target/AMDGPU/Disassembler/AMDGPUDisassembler.cpp
643–644

This looks as a small and acceptable hack to me. Other alternatives would require more changes without significant benefits.

dp accepted this revision.May 25 2022, 11:55 AM

LGTM. I believe the issue with unused MCInst fields may be fixed by a separate patch (if this is really necessary).

This revision is now accepted and ready to land.May 25 2022, 11:55 AM
This revision was landed with ongoing or failed builds.May 25 2022, 12:12 PM
This revision was automatically updated to reflect the committed changes.