[PATCH] D120855: [AMDGPU] gfx940 uses new names for coherency bits

Mon Mar 7 10:23:18 PST 2022

kzhuravl accepted this revision.
kzhuravl added a comment.
This revision is now accepted and ready to land.

Other than a TODO comment, LGTM, thanks

================
Comment at: llvm/lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp:4263

-  if (isGFX90A() && (CPol & CPol::SCC)) {
+  if (isGFX90A() && !isGFX940() && (CPol & CPol::SCC)) {
     SMLoc S = getImmLoc(AMDGPUOperand::ImmTyCPol, Operands);
----------------
rampitec wrote:
> critson wrote:
> > 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?
> 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.
Can we add a TODO mentioning that this needs a cleanup?

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D120855/new/

https://reviews.llvm.org/D120855