[PATCH] D137952: [AMDGPU][GFX11] Refactor VOPD operands handling

[Dmitry Preobrazhensky via Phabricator via llvm-commits]

dp added a comment.

> Why does it change the error position?

Before this change `getParsedSrcIndex` handled tied operands as if they were parsed and returned dst index for tied operands:

  unsigned getParsedSrcIndex(unsigned SrcIdx, bool ComponentHasSrc2Acc) const {
    if (ComponentHasSrc2Acc && SrcIdx == (MAX_SRC_NUM - 1))
      return getParsedDstIndex();
    ...
  }

This patch corrects handling of parsed operands: now they don't include tied ones.



================
Comment at: llvm/lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp:8501
+    auto ParsedSrcOperandsNum = InstInfo[CompIdx].getParsedSrcOperandsNum();
+    for (unsigned SrcIdx = 0; SrcIdx < ParsedSrcOperandsNum; ++SrcIdx)
+      addOp(CInfo.getParsedSrcIndex(SrcIdx));
----------------
Joe_Nash wrote:
> Can we rename SrcIdx to something like MCSrcIdx? So that there is no confusion with the ParsedSrcIndex returned by getParsedSrcIndex.
I agree that there is some confusion with naming. It should be clear when the code operates with indices to opcode operands, indices to parsed operands and indices to MC operands.

But `MC` prefix would suggest iteration over MCInst operands, would it not? Maybe `OpSrcIdx` would be a better choice?



================
Comment at: llvm/lib/Target/AMDGPU/Utils/AMDGPUBaseInfo.cpp:538
+    RegIndices[OprIdx] = Comp.hasRegSrcOperand(SrcIdx)
+                             ? GetRegIdx(ComponentIdx, Comp.getSrcIndex(SrcIdx))
+                             : 0;
----------------
Another problem with naming is here: `getSrcIndex(SrcIdx)` looks awful to me. How about renaming `getSrc*` (and other similar functions) to `getMCSrc*`?


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

https://reviews.llvm.org/D137952