Enable Intel AVX512_FP16 instructions and test

[Cui, Lili]

> -----Original Message-----
> From: Jan Beulich <jbeulich@suse.com>
> Sent: Wednesday, August 4, 2021 11:50 PM
> To: Cui, Lili <lili.cui@intel.com>
> Cc: hjl.tools@gmail.com; binutils@sourceware.org
> Subject: Re: Enable Intel AVX512_FP16 instructions and test
> 
> On 04.08.2021 16:47, Cui, Lili wrote:
> > @@ -6088,6 +6092,21 @@ check_VecOperands (const insn_template *t)
> >        return 1;
> >      }
> >
> > +  /* For some special instructions require that destination must be distinct
> > +     from source registers.  */
> > +  if (t->opcode_modifier.reqdistinctreg && i.operands >= 3)
> 
> I'd like to suggest that "i.operands >= 3" be taken out of there and converted to
> e.g. "know (i.operands >= 3)" inside the if()'s body.
> 
Good idea! Done.

> > +    {
> > +      unsigned int dest_reg = i.operands - 1;
> > +
> > +      /* #UD if dest_reg == src1_reg or dest_reg == src2_reg.  */
> > +      if (i.op[dest_reg - 1].regs == i.op[dest_reg].regs
> > +	  || (i.reg_operands > 2
> > +	      && i.op[dest_reg - 2].regs == i.op[dest_reg].regs))
> > +	{
> > +	  i.error = invalid_dest_and_src_register_set;
> > +	  return 1;
> > +	}
> > +    }
> >    /* Check if broadcast is supported by the instruction and is applied
> >       to the memory operand.  */
> >    if (i.broadcast.type)
> 
> Can you please retain a blank line between the unrelated code blocks?
> 
Done.

> > @@ -7628,6 +7650,16 @@ check_word_reg (void)
> >  		i.suffix);
> >  	return 0;
> >        }
> > +    /* For some instructions need encode as EVEX.W=1 without explicit
> VexW1. */
> > +    else if (i.types[op].bitfield.qword
> > +	     && i.tm.operand_types[op].bitfield.class == Reg
> > +	     && intel_syntax
> > +	     && i.tm.opcode_modifier.toqword
> > +	     && i.types[0].bitfield.class != RegSIMD)
> 
> Largely out of curiosity, is the last part of the condition actually needed?
> 
There are two judgments in this place that are really redundant, I changed it to this

    else if (i.types[op].bitfield.qword
             && intel_syntax
             && i.tm.opcode_modifier.toqword)
      {
          /* Convert to QWORD.  We want EVEX.W byte. */
          i.suffix = QWORD_MNEM_SUFFIX;
      }

> > +      {
> > +          /* Convert to QWORD.  We want EVEX.W byte. */
> > +	  i.suffix = QWORD_MNEM_SUFFIX;
> 
> Nit: Indentation of the two lines doesn't match.
> 
Done.

> > @@ -13507,6 +13665,45 @@ MOVSXD_Fixup (int bytemode, int sizeflag)
> >    OP_E (bytemode, sizeflag);
> >  }
> >
> > +ReqDistinctReg_Fixup (int bytemode, int sizeflag)
> 
> Please don't omit the "static void" here. I'm actually surprised that the compiler
> didn't warn about this form.
> 
Done.

> > +{
> > +  unsigned int reg = vex.register_specifier;
> > +  unsigned int modrm_reg = modrm.reg;
> > +  unsigned int modrm_rm = modrm.rm;
> > +
> > +  /* Calc destination register number.  */  if (rex & REX_R)
> > +    modrm_reg += 8;
> > +  if (!vex.r)
> > +    modrm_reg += 16;
> > +
> > +  /* Calc src1 register number.  */
> > +  if (address_mode != mode_64bit)
> > +    reg &= 7;
> > +  else if (vex.evex && !vex.v)
> > +    reg += 16;
> > +
> > +  /* Calc src2 register number.  */
> > +  if (modrm.mod == 3)
> > +    {
> > +      if (rex & REX_B)
> > +        modrm_rm += 8;
> > +      if ((rex & REX_X))
> 
> Ideally you'd be consistent with the use of parentheses in similar statements.
> 
Done.

Thanks,
Lili.