[PATCH] D96469: [AMDGPU] WIP: use single cache policy operand

[Dmitry Preobrazhensky via Phabricator via llvm-commits]

dp added a comment.

Overall looks good



================
Comment at: llvm/lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp:3839
+  if (TSFlags & SIInstrFlags::IsAtomicRet) {
+    if (CPolPos == -1 || !(Inst.getOperand(CPolPos).getImm() & CPol::GLC)) {
+      Error(IDLoc, "instruction must use glc");
----------------
Should be removed


================
Comment at: llvm/lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp:4904
+  // since parseOptionalOperand just consumed all of the individual bits.
+  if (ResTy == MatchOperand_Success &&
+      (Mnemonic.startswith("scratch_") || Mnemonic.startswith("flat_") ||
----------------
Should also combine cache policy operands on MatchOperand_NoMatch condition, otherwise these operands may break operand matcher later and may result in an invalid error position. For example:

    error: invalid operand for instruction
    global_atomic_add v0, v[1:2], v2, off glc 1
                                          ^

Hopefully the whole hack will be removed in final version.


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

https://reviews.llvm.org/D96469