[PATCH] D90730: [AMDGPU] Add default 1 glc operand to rtn atomics

[Stanislav Mekhanoshin via Phabricator via llvm-commits]

rampitec marked an inline comment as done.
rampitec added inline comments.


================
Comment at: llvm/lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp:337-338
   bool isDLC() const { return isImmTy(ImmTyDLC); }
   bool isGLC() const { return isImmTy(ImmTyGLC); }
+  bool isGLC_1() const { return isImmTy(ImmTyGLC); }
   bool isSLC() const { return isImmTy(ImmTySLC); }
----------------
scott.linder wrote:
> Is the `_1` variant added here rather than replacing the existing one to support an old syntax, or to support disjoint classes of instructions which need different defaults/validation, or something else? I'm not sure I understand, but renaming the `_1` to something more descriptive or adding a comment might help. It also seems like the other `isX` functions are all checking for something distinct (i.e. there is `ImmTyOffset`, `ImmTyOffset0`, and `ImmTyOffset1` above), is that not needed here?
This is MatchClass of the GLC_1 operand, and GLC_1 operand is added the same way as GLC_0 was added, this is the default (and in this case forced) value of the GLC operand. It literally matches GLC_1 operand in the instruction inputs. Added comment.


================
Comment at: llvm/lib/Target/AMDGPU/Disassembler/AMDGPUDisassembler.cpp:401
+  if (Res && (MCII->get(MI.getOpcode()).TSFlags &
+                        (SIInstrFlags::MTBUF | SIInstrFlags::MUBUF |
+                         SIInstrFlags::FLAT)) &&
----------------
dp wrote:
> Could MTBUF be removed?
Yep, at the end MTBUF does not need it as it does not have atomics.


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

https://reviews.llvm.org/D90730