This is an archive of the discontinued LLVM Phabricator instance.

[AMDGPU][MC][NFC] Refactored sendmsg(...) handling
ClosedPublic

Authored by dp on Mar 18 2022, 4:39 AM.

Details

Summary

Switched to using new custom operands design.
See https://reviews.llvm.org/D121696.

Diff Detail

Event Timeline

dp created this revision.Mar 18 2022, 4:39 AM
Herald added a project: Restricted Project. · View Herald TranscriptMar 18 2022, 4:39 AM
dp requested review of this revision.Mar 18 2022, 4:39 AM
Herald added a project: Restricted Project. · View Herald TranscriptMar 18 2022, 4:39 AM
foad added inline comments.Mar 18 2022, 4:47 AM
llvm/lib/Target/AMDGPU/Utils/AMDGPUAsmUtils.cpp
24

I think it would be neater to define and use a new isGFX8GFX9 instead of having a lambda inside this table. (And the same for the hwreg table that you pushed already.)

dp updated this revision to Diff 416471.Mar 18 2022, 5:26 AM

Corrected as suggested by Jay.

This revision is now accepted and ready to land.Mar 18 2022, 10:19 AM
foad added a comment.Mar 21 2022, 3:43 AM

LGTM (with or without addressing my nitpicks about naming).

llvm/lib/Target/AMDGPU/Utils/AMDGPUBaseInfo.cpp
1500

Maybe it would be simpler to implement this as "isNotGFX10Plus" instead?

1518

In AMDGPU.td there is a predicate called "isGFX10Before1030". Maybe we should use the same name in both places? I don't have a preference for which name is better.

dp marked 2 inline comments as done.Mar 21 2022, 4:28 AM
dp added inline comments.
llvm/lib/Target/AMDGPU/Utils/AMDGPUBaseInfo.cpp
1500

Thanks, corrected.

1518

I'd have preferred isGFX10RDNA1 but I do not want to correct code that is not related to this change. So I'll use isGFX10Before1030 as you suggested.

This revision was landed with ongoing or failed builds.Mar 21 2022, 5:38 AM
This revision was automatically updated to reflect the committed changes.
dp marked 2 inline comments as done.