[PATCH] D136629: [AMDGPU] Fix delay alu for VOPD with src2acc

[Dmitry Preobrazhensky via Phabricator via llvm-commits]

dp added a comment.

Overall looks fine, but I want somebody to look at codegen changes. You could leave MC code refactoring to me (but I'm not sure if I'll be able to improve code that much).



================
Comment at: llvm/lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp:8551
     for (unsigned SrcIdx = 0; SrcIdx < SrcOperandsNum; ++SrcIdx) {
-      addOp(InstInfo[CompIdx].getParsedSrcIndex(SrcIdx));
+      auto CInfo = InstInfo[CompIdx];
+      addOp(CInfo.getParsedSrcIndex(SrcIdx, CInfo.hasSrc2Acc()));
----------------
This may be moved out of the loop (together with `CInfo.hasSrc2Acc()`).


================
Comment at: llvm/lib/Target/AMDGPU/Utils/AMDGPUBaseInfo.cpp:540
   unsigned Src2Reg = 0;
   if (Comp.hasRegularSrcOperand(2))
     Src2Reg = GetRegIdx(ComponentIdx, Comp.getSrcIndex(2));
----------------
`hasRegularSrcOperand` looks counterintuitive now because tied src2 is actually a quite special operand. Counting this operand in `ComponentProps.SrcOperandsNum` is also debatable. Maybe handling this operand explicitly would make code more clear. It will also make error position correct. But it will require changes in parser and codegen which I believe you tried to avoid.


================
Comment at: llvm/lib/Target/AMDGPU/Utils/AMDGPUBaseInfo.h:580
   }
-  unsigned getParsedSrcIndex(unsigned SrcIdx) const {
+  unsigned getParsedSrcIndex(unsigned SrcIdx, bool ComponentHasSrc2Acc) const {
     assert(SrcIdx < Component::MAX_SRC_NUM);
----------------
I believe this change may be avoided by making `ComponentLayout` a subclass of `ComponentProps`.


================
Comment at: llvm/test/MC/AMDGPU/gfx11_asm_vopd_errs.s:269
 // GFX11-NEXT:{{^}}v_dual_fmamk_f32    v6, v1, 0xaf123456, v3       :: v_dual_fmac_f32       v5, v2, v3
-// GFX11-NEXT:{{^}}                                        ^
+// GFX11-NEXT:{{^}}                                                                          ^
----------------
It's a pity.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136629