FW: [PATCH 1/2] [PATCH 1/2] Enable Intel AVX512_FP16 instructions

[Cui, Lili]

> -----Original Message-----
> From: Jan Beulich <jbeulich@suse.com>
> Sent: Wednesday, July 14, 2021 11:21 PM
> To: Cui, Lili <lili.cui@intel.com>
> Cc: hjl.tools@gmail.com; binutils@sourceware.org
> Subject: Re: [PATCH 1/2] [PATCH 1/2] Enable Intel AVX512_FP16 instructions
> 
> On 13.07.2021 08:58, Cui, Lili wrote:
> 
> Disassembler:
> 
> d_scalar_mode looks to be unused.
>
> This
> 
>   /* EVEX_W_MAP5_2A_P_1 */
>   {
>     { "vcvtsi2sh{%LQ|}",	{ XMScalar, VexScalar, EXxEVexR, Ed }, 0 },
>     { "vcvtsi2sh{%LQ|}",	{ XMScalar, VexScalar, EXxEVexR, Eq }, 0 },
>   },
> 
> can imo be expressed without decoding EVEX.W, by using Edq instead of
> (separately) Ed and Eq. There's at least one similar case elsewhere.
> Interestingly in the 2si/2usi conversions you do use Gdq already, which I
> think handles the EVEX.W=1 case correctly outside of 64-bit mode (unlike Eq,
> which will unconditionally produce 64-bit register names afaict).
> 
> As to a broader question on decoding EVEX.W: Did you consider introducing
> e.g. %XH (paralleling %XW, just that EVEX.W=1 is not a valid encoding), to
> avoid this decode step for perhaps almost all entries? And if that's not an
> option, decoding EVEX.W first for all the opcodes which previously had no
> meaning at all would, in some cases, reduce the overall number of table
> entries (and in all other cases this would then merely be for consistency, as it
> also wouldn't increase the number of table entries). To give an example:
> 
>     { PREFIX_TABLE (PREFIX_EVEX_0F3AC2) },
> 
> =>
> 
>   /* PREFIX_EVEX_0F3AC2 */
>   {
>     { VEX_W_TABLE (EVEX_W_0F3AC2_P_0) },
>     { VEX_W_TABLE (EVEX_W_0F3AC2_P_1) },
>   },
> 
> =>
> 
>   /* EVEX_W_0F3AC2_P_0 */
>   {
>     { "vcmpph",	{ XMask, Vex, EXxh, EXxEVexS, Ib }, 0 },
>   },
>   /* EVEX_W_0F3AC2_P_1 */
>   {
>     { "vcmpsh",	{ XMask, VexScalar, EXxmm_mw, EXxEVexS, Ib }, 0 },
>   },
> 
> i.e. a total of 1 + 4 + 2 * 2 entries. Whereas decoding W first would yield 1
> (evex) + 2 (evex_w) + 4 (prefix) entries. 

Hi Jan,

Do you want me to change it like this?
     { PREFIX_TABLE (PREFIX_EVEX_0F3AC2) },
 
 =>
 
   /* PREFIX_EVEX_0F3AC2 */
   {
     { "vcmp%XH",	{ XMask, Vex, EXxh, EXxEVexS, Ib }, 0 },
     { "vcmp%XH",	{ XMask, VexScalar, EXxmm_mw, EXxEVexS, Ib }, 0 },
   },

"XH" => print 'ph', 'sh' depending on the EVEX.ll bit, if EVEX.W==W1 report bad code.
if  (EVEX.LL== EVEX.LLIG)
      print 'sh'   
else 
      print 'ph'

I do not quite understand it, if we change the disassembler like this, when we add a new instruction with same opcode and EVEX.W==W1, we need to change the old one.


>The delta is even larger for something
> like MAP5_7D: 1 + 4 + 4 * 2 vs. 1 + 2 + 4. This also results in more related
> entries ending up closer to one another.
> 
I don't quite understand here,  should I let all FP16 disassembler go through W_TABLE fist? or just add something like %XH instead of going through W_TABLE? Thanks.


> As to formatting, it looks as if the first hunk changing
> intel_operand_size() mis-indents the return statement that was there
> already before.
> 
> Jan