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

[Joe Nash via Phabricator via llvm-commits]

Joe_Nash added inline comments.


================
Comment at: llvm/lib/Target/AMDGPU/Utils/AMDGPUBaseInfo.cpp:540
   unsigned Src2Reg = 0;
   if (Comp.hasRegularSrcOperand(2))
     Src2Reg = GetRegIdx(ComponentIdx, Comp.getSrcIndex(2));
----------------
dp wrote:
> `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.
Yes, I have tried to avoid special case code in the parser and codegen. Counting the operand in SrcOperandsNum is indeed debatable but  it is a standard tied operand from a CodeGen perspective and the VOPD rules are the same for dst and src2, so it works out cleanly. I'm open to an NFC (except for error caret position) refactor in the future.


================
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);
----------------
dp wrote:
> I believe this change may be avoided by making `ComponentLayout` a subclass of `ComponentProps`.
It seems natural to delete ComponentLayout and incorporate its functionaliy into ComponentInfo. I'd prefer that was in a separate patch. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136629