[PATCH] D128218: [AMDGPU] gfx11 VOPD instructions MC support

[Joe Nash via Phabricator via llvm-commits]

Joe_Nash added inline comments.


================
Comment at: llvm/lib/Target/AMDGPU/VOP2Instructions.td:290
 
+multiclass VOP2eInst_VOPD <string opName,
+                      VOPProfile P,
----------------
dp wrote:
> Is there a way to avoid code duplication here? `VOP2eInst` and `VOP2eInst_VOPD` are almost identical.
Good point, I have deduplicated it.


================
Comment at: llvm/lib/Target/AMDGPU/VOPDInstructions.td:58
+class VOPD_Base<dag outs, dag ins, string asm, VOP_Pseudo VDX,
+           VOP_Pseudo VDY, VOPD_Component XasVC, VOPD_Component YasVC>
+    : VOPAnyCommon<outs, ins, asm, []>,
----------------
dp wrote:
> Indent.
> What does `VC` stand for?
VOPD_Component. 


================
Comment at: llvm/lib/Target/AMDGPU/VOPInstructions.td:37
+  bits<5> VOPDOp = OpIn;
+  bit CanBeVOPDX = !le(VOPDOp, 13);
+}
----------------
dp wrote:
> Could we avoid hardcoding `13` here? Ideally it should be defined together with `VOPDXPseudos` and `VOPDYPseudos`.
Yes, I have made it a named variable instead, used here and in defvar VOPDXPseudos. In theory I could move VOPDXPseudos and VOPDY pseudos here and call !size on it, but that seems slightly worse for file encapsulation purposes


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128218