This is an archive of the discontinued LLVM Phabricator instance.

[AMDGPU] gfx940 uses new names for coherency bits
ClosedPublic

Authored by rampitec on Mar 2 2022, 4:13 PM.

Diff Detail

Event Timeline

rampitec created this revision.Mar 2 2022, 4:13 PM
Herald added a project: Restricted Project. · View Herald TranscriptMar 2 2022, 4:13 PM
rampitec requested review of this revision.Mar 2 2022, 4:13 PM
Herald added a project: Restricted Project. · View Herald TranscriptMar 2 2022, 4:13 PM
Herald added a subscriber: wdng. · View Herald Transcript
critson added a subscriber: critson.Mar 2 2022, 8:04 PM
critson added inline comments.
llvm/lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp
4263

This code looks odd because from the naming alone I would expect isGFX90A and isGFX940 to be mutually exclusive.
Should those test really be named hasGFX90A_Insts, or be actually made exclusive?

rampitec added inline comments.Mar 2 2022, 9:47 PM
llvm/lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp
4263

This is true, but this is all across the MC layer and many places above it, not just for this target. It can use a separate and massive cleanup, not simply changing this line.

kzhuravl accepted this revision.Mar 7 2022, 10:23 AM

Other than a TODO comment, LGTM, thanks

llvm/lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp
4263

Can we add a TODO mentioning that this needs a cleanup?

This revision is now accepted and ready to land.Mar 7 2022, 10:23 AM
rampitec updated this revision to Diff 413549.Mar 7 2022, 10:29 AM
rampitec marked an inline comment as done.

Added TODO at isGFX90A() declaration.

This revision was landed with ongoing or failed builds.Mar 7 2022, 11:50 AM
This revision was automatically updated to reflect the committed changes.