[PATCH v5] Support ymm rounding control for Intel AVX10.2

[Jiang, Haochen]

> From: Jan Beulich <jbeulich@suse.com>
> Sent: Monday, August 26, 2024 10:23 PM
> 
> On 23.08.2024 10:38, Haochen Jiang wrote:
> > In the patch, in order to support ymm rounding for AVX10.2, we derive
> > evex attribute for all cases instead of only for rc_none to encode U bit.
> > Also changed some bad_opcode return due to the share of U bit with APX_F.
> >
> > Instead of adding new table entries, our current method is to alter the
> > CPUID part to add AVX10_2 explicitly for ymm rounding support, also
> > using similar way as APX_F for the CPUID combination, since ymm rounding
> > control is also somehow an instruction promotion, i.e.:
> >
> >   #define AVX10_2(cpuid) cpuid&(cpuid|AVX10_2)
> 
> The APX attribute was only added as necessary. Legacy insns being extended
> to use REX2 encodings weren't altered. Similarly no AVX10.1 attributes were
> added anywhere. IOW as before - please limit the opcode table changes to
> just those places where new insn forms need adding (I expect that's going
> to be a couple of convert insns with differring source/destination element
> sizes).

AVX10.1 definitely doesn't need to be added everywhere since we are actually
implying AVX512.

However, AVX10.2 is different, it does contain new feature for ymm rounding.
I am not changing everywhere but only altering those related instruction CPUID,
which is the maybe optimal change for now since splitting encoding_evex512
will be a separate patch. All of them are needed, which are actually the new insn
forms since ymm rounding doesn't exist before AVX10.2.

> 
> > +  u = (i.rounding.type != rc_none
> > +       && i.tm.opcode_modifier.evex == EVEX256) ? false : true;
> 
> What's the conditional operator for? This
> 
>   u = (i.rounding.type == rc_none
>        || i.tm.opcode_modifier.evex != EVEX256);
> 
> is imo easier to follow.

It comes from original 0-1. Yes, it should be that after changing to bool.

> 
> > +	 enabled.  There's no need to check all operands, though: Either of the
> >  	 last two operands will be of the right size in all relevant templates.  */
> >        if (t->opcode_modifier.evex != EVEXLIG
> >  	  && t->opcode_modifier.evex != EVEX512
> > +	  && (t->opcode_modifier.evex != EVEX256
> > +	      || !cpu_arch_flags.bitfield.cpuavx10_2)
> >  	  && !i.types[t->operands - 1].bitfield.zmmword
> > -	  && !i.types[t->operands - 2].bitfield.zmmword)
> > +	  && !i.types[t->operands - 2].bitfield.zmmword
> > +	  && ((!i.types[t->operands - 1].bitfield.ymmword
> > +	       && !i.types[t->operands - 2].bitfield.ymmword)
> > +	      || !cpu_arch_flags.bitfield.cpuavx10_2))
> >  	{
> >  	  i.error = operand_size_mismatch;
> >  	  return 1;
> 
> Since you now uniformly resolve t->opcode_modifier.evex when it's EVEXDYN,
> do you actually still need the .zmmword / .ymmword checks? In the
> calculation of the U bit you also rely on solely that attribute.

check_VecOperands() comes earlier than build_evex_prefix(). At this point,
the evex for EVEXDYN has not been derived. We could and I have considered to
move that part into check_VecOperands(), but I suppose it might be out of
scope for this function.

Thx,
Haochen

> 
> Jan