[PATCH] D129637: [AMDGPU][MC][GFX11] Correct disassembly of *_e64_dpp opcodes which support op_sel

[Petar Avramovic via Phabricator via llvm-commits]

Petar.Avramovic added inline comments.


================
Comment at: llvm/lib/Target/AMDGPU/Disassembler/AMDGPUDisassembler.cpp:782
+             AMDGPU::getNamedOperandIdx(Opc, AMDGPU::OpName::op_sel) != -1) {
+    insertNamedMCOperand(MI, MCOperand::createImm(getOpSel(MI)),
+                         AMDGPU::OpName::op_sel);
----------------
You don't need to calculate constant to put in opsel operand in disassembler, opsel bits are already set in src_modifiers, inst_printer does not access op_sel operand. You can simply use 0.
You only need to add op_sel operand so that other two operands dpp8 and fi are at correct index.

AsmParser uses op_sel operand to parse opsel and then uses that operand to set opsel bit in src_modifiers. Because Srco operands are parsed first, opsel is parsed after all src operands. After opsel bits in src modifiers are set (e.g. after cvtVOP3OpSel or cvtVOP3P) constant value inside opsel operand could be set to 0 since it would not be used any more.


================
Comment at: llvm/lib/Target/AMDGPU/VOPInstructions.td:1289
     : VOP3_DPP8<op, opName, ps.Pfl> {
+  let VOP3_OPSEL = ps.Pfl.HasOpSel;
   let hasSideEffects = ps.hasSideEffects;
----------------
Is this to allow printing of opsel[3] (SISrcMods::DST_OP_SEL in src modifiers)?
btw opsel is not properly parsed for dot8/dot16 (cvtVOP3DPP)


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

https://reviews.llvm.org/D129637