[PATCH] D132957: [AMDGPU][MC][GFX11][NFC] Update tests for VOP3P.DPP instructions

Wed Aug 31 07:13:56 PDT 2022

Joe_Nash added inline comments.

================
Comment at: llvm/test/MC/AMDGPU/gfx11_asm_vop3p_dpp16.s:20
+
+// FIXME: it is not clear if op_sel may be used with these instructions. SPG requires op_sel=0 and op_sel_hi=7.
+
----------------
dp wrote:
> Joe_Nash wrote:
> > I'm not sure where you're getting op_sel_hi=7 for v_fma_mix* instructions. From my reading of the spec and comparison with sp3, all values of op_sel and op_sel_hi are supported. The behavior is non-standard for mix opcodes, but the values should be accepted. 
> > Does your observation have anything to do with dpp? Both dpp and non-dpp version should behave the same and allow all values of op_sel. 
> I remember that SPG stated the following at some point: 
> 
>     When using DPP with VOP3/VOP3P, the OPSEL must be set such that the low result only uses low inputs, and the high result only uses high inputs.
> 
> I always thought that this was a suspicious statement, but see the original tests `gfx11_asm_vop3p_dpp8.s` below, they stated essentially the same:
> 
>     // For test purpose only. OP_SEL has to be set to all 0 and OP_SEL_HI has to be set to all 1
Oh I see. In my previous reply I was just looking at the rules for vop3p by mistake. The existing comment was actually correct. There is a special case for vop3p with dpp. 

When using DPP with VOP3P, OPSEL has to be set to all 0 and OPSEL_HI has to be set to all 1

And in GCNDPPCombine we actually check those rules. So Codegen is doing the right thing.

Sorry for the noise. 

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

https://reviews.llvm.org/D132957