Home Original page

Enable Intel AVX512_FP16 instructions and test

Cui, Lili lili.cui@intel.com
Wed Aug 4 14:40:27 GMT 2021 
> -----Original Message-----
> From: Jan Beulich <jbeulich@suse.com>
> Sent: Tuesday, August 3, 2021 10:23 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 30.07.2021 11:00, Cui, Lili wrote:
> > Rebased AVX512-FP16 patch and added error checking for some
> > instructions that require different destination and source registers
> 
> I can't seem to be able to spot respective disassembler changes? (I have to
> also admit that the restriction is quite well hidden in the spec - there's no
> mention of it except in the "Additionally:" sections. I'd have expected such
> special cases to be mentioned in "Operation" as well.)
> 
Add distinct register check for disassembler and also add test case for it, thanks.

> I'm not sure a new insn attribute is warranted here (iirc you got away
> without for the AMX special restrictions), but I also don't really want to
> request that you redo this. What I would like to see improved though is the
> name: It would better express that it's the destination that needs to be
> distinct (unlike for the AMX insns, where all registers need to be distinct).
> Also the error message wording "destination and source registers must be
> distinct" is ambiguous (one may read it to mean the same as what the AMX
> requirement is). I'd suggest "destination must be distinct from source
> registers".
> 
Done.

> > @@ -1078,6 +1093,48 @@ enum
> >    PREFIX_EVEX_0F389B,
> >    PREFIX_EVEX_0F38AA,
> >    PREFIX_EVEX_0F38AB,
> > +
> > +  PREFIX_EVEX_0F3A08_W_0,
> > +  PREFIX_EVEX_0F3A0A_W_0,
> > +  PREFIX_EVEX_0F3A26,
> > +  PREFIX_EVEX_0F3A27,
> > +  PREFIX_EVEX_0F3A56,
> > +  PREFIX_EVEX_0F3A57,
> > +  PREFIX_EVEX_0F3A66,
> > +  PREFIX_EVEX_0F3A67,
> > +  PREFIX_EVEX_0F3AC2,
> > +
> > +  PREFIX_EVEX_MAP5_10,
> > +  PREFIX_EVEX_MAP5_11,
> > +  PREFIX_EVEX_MAP5_1D,
> > +  PREFIX_EVEX_MAP5_2A,
> > +  PREFIX_EVEX_MAP5_2C,
> > +  PREFIX_EVEX_MAP5_2D,
> > +  PREFIX_EVEX_MAP5_2E,
> > +  PREFIX_EVEX_MAP5_2F,
> > +  PREFIX_EVEX_MAP5_51,
> > +  PREFIX_EVEX_MAP5_58,
> > +  PREFIX_EVEX_MAP5_59,
> > +  PREFIX_EVEX_MAP5_5A_W_0,
> > +  PREFIX_EVEX_MAP5_5A_W_1,
> > +  PREFIX_EVEX_MAP5_5B_W_0,
> > +  PREFIX_EVEX_MAP5_5B_W_1,
> > +  PREFIX_EVEX_MAP5_5C,
> > +  PREFIX_EVEX_MAP5_5D,
> > +  PREFIX_EVEX_MAP5_5E,
> > +  PREFIX_EVEX_MAP5_5F,
> > +  PREFIX_EVEX_MAP5_78,
> > +  PREFIX_EVEX_MAP5_79,
> > +  PREFIX_EVEX_MAP5_7A,
> > +  PREFIX_EVEX_MAP5_7B,
> > +  PREFIX_EVEX_MAP5_7C,
> > +  PREFIX_EVEX_MAP5_7D_W_0,
> > +
> > +  PREFIX_EVEX_MAP6_13,
> > +  PREFIX_EVEX_MAP6_56,
> > +  PREFIX_EVEX_MAP6_57,
> > +  PREFIX_EVEX_MAP6_D6,
> > +  PREFIX_EVEX_MAP6_D7
> >  };
> 
> Still no trailing comma here?
> 
Sorry, added it.

> > @@ -1159,7 +1216,9 @@ enum
> >  {
> >    EVEX_0F = 0,
> >    EVEX_0F38,
> > -  EVEX_0F3A
> > +  EVEX_0F3A,
> > +  EVEX_MAP5,
> > +  EVEX_MAP6
> >  };
> 
> And here?
> 
Done.
> > @@ -1647,6 +1722,7 @@ struct dis386 {
> >     "XZ" => print 'x', 'y', or 'z' if suffix_always is true or no
> >  	   register operands and no broadcast.
> >     "XW" => print 's', 'd' depending on the VEX.W bit (for FMA)
> > +   "XH" => print 'h' if EVEX.W=0, EVEX.W=1 is not a valid
> > + encoding(for FP16)
> 
> Nit: Please add a blank ahead of the opening parenthesis.
> 
Done.

> > @@ -467,6 +470,8 @@ enum
> >    Size,
> >    /* check register size.  */
> >    CheckRegSize,
> > +  /* check register number, registers should be distinct.  */
> > + ReqDistinctReg,
> 
> I'm afraid the comment is misleading.
> 
Modified it.

Thanks,
Lili.
More information about the Binutils mailing list