x86: fold AVX512-VNNI disassembler entries with AVX-VNNI ones
Checks
Commit Message
Make %XV also print the separating blank in the VEX case, while making
it do nothing for EVEX-encoded insns. This way the AVX-VNNI entries
can be re-used for AVX512-VNNI, at the same time fixing the lack of
EVEX.W decoding.
For the AVX-VNNI ones further make sure only VEX.66 forms are actually
decoded.
---
Irrespective of this change I continue to disagree with the arbitrary
printing of "{vex}" for the AVX-VNNI insns: If that's meant for
disambiguation purposes, then EVEX-encoded insns not using EVEX-specific
functionality by having VEX counterparts (vaddps %xmm0, %xmm0, %xmm0)
should also be prefixed by "{evex}".
Comments
On Fri, Oct 14, 2022 at 3:22 AM Jan Beulich <jbeulich@suse.com> wrote:
>
> Make %XV also print the separating blank in the VEX case, while making
> it do nothing for EVEX-encoded insns. This way the AVX-VNNI entries
> can be re-used for AVX512-VNNI, at the same time fixing the lack of
> EVEX.W decoding.
>
> For the AVX-VNNI ones further make sure only VEX.66 forms are actually
> decoded.
> ---
> Irrespective of this change I continue to disagree with the arbitrary
> printing of "{vex}" for the AVX-VNNI insns: If that's meant for
> disambiguation purposes, then EVEX-encoded insns not using EVEX-specific
> functionality by having VEX counterparts (vaddps %xmm0, %xmm0, %xmm0)
> should also be prefixed by "{evex}".
This is done to match the assembler. There are 3 kinds of VNNI processors:
1. AVX512-VNNI only.
2. AVX-VNNI only.
3. AVX512-VNNI and AVX-VNNI.
Since AVX512-VNNI came out first, all VNNI instructions without a prefix
are encoded as AVX512-VNNI. The existing VNNI instructions without
a prefix, generated by compiler or hand written, are encoded with EVEX.
If one needs VNNI with VEX encoding, the {vex} prefix should be used.
This applies to any AVX extensions which come after EVEX ones, including
AVX-IFMA.
> --- a/opcodes/i386-dis.c
> +++ b/opcodes/i386-dis.c
> @@ -1755,7 +1755,7 @@ struct dis386 {
> "XD" => print 'd' if !EVEX or EVEX.W=1, EVEX.W=0 is not a valid encoding
> "XH" => print 'h' if EVEX.W=0, EVEX.W=1 is not a valid encoding (for FP16)
> "XS" => print 's' if !EVEX or EVEX.W=0, EVEX.W=1 is not a valid encoding
> - "XV" => print "{vex3}" pseudo prefix
> + "XV" => print "{vex} " pseudo prefix
> "LQ" => print 'l' ('d' in Intel mode) or 'q' for memory operand, cond
> being false, or no operand at all in 64bit mode, or if suffix_always
> is true.
> @@ -7545,19 +7545,19 @@ static const struct dis386 vex_w_table[]
> },
> {
> /* VEX_W_0F3850 */
> - { "%XV vpdpbusd", { XM, Vex, EXx }, 0 },
> + { "%XVvpdpbusd", { XM, Vex, EXx }, PREFIX_DATA },
> },
> {
> /* VEX_W_0F3851 */
> - { "%XV vpdpbusds", { XM, Vex, EXx }, 0 },
> + { "%XVvpdpbusds", { XM, Vex, EXx }, PREFIX_DATA },
> },
> {
> /* VEX_W_0F3852 */
> - { "%XV vpdpwssd", { XM, Vex, EXx }, 0 },
> + { "%XVvpdpwssd", { XM, Vex, EXx }, PREFIX_DATA },
> },
> {
> /* VEX_W_0F3853 */
> - { "%XV vpdpwssds", { XM, Vex, EXx }, 0 },
> + { "%XVvpdpwssds", { XM, Vex, EXx }, PREFIX_DATA },
> },
> {
> /* VEX_W_0F3858 */
> @@ -10711,22 +10711,29 @@ putop (instr_info *ins, const char *in_t
> case 'V':
> if (l == 0)
> abort ();
> - else if (l == 1
> - && (last[0] == 'L' || last[0] == 'X'))
> + else if (l == 1)
> {
> - if (last[0] == 'X')
> + switch (last[0])
> {
> + case 'X':
> + if (ins->vex.evex)
> + break;
> *ins->obufp++ = '{';
> *ins->obufp++ = 'v';
> *ins->obufp++ = 'e';
> *ins->obufp++ = 'x';
> *ins->obufp++ = '}';
> - }
> - else if (ins->rex & REX_W)
> - {
> + *ins->obufp++ = ' ';
> + break;
> + case 'L':
> + if (!(ins->rex & REX_W))
> + break;
> *ins->obufp++ = 'a';
> *ins->obufp++ = 'b';
> *ins->obufp++ = 's';
> + break;
> + default:
> + abort ();
> }
> }
> else
> --- a/opcodes/i386-dis-evex.h
> +++ b/opcodes/i386-dis-evex.h
> @@ -383,8 +383,8 @@ static const struct dis386 evex_table[][
> { "vrsqrt14p%XW", { XM, EXx }, 0 },
> { "vrsqrt14s%XW", { XMScalar, VexScalar, EXdq }, PREFIX_DATA },
> /* 50 */
> - { "vpdpbusd", { XM, Vex, EXx }, PREFIX_DATA },
> - { "vpdpbusds", { XM, Vex, EXx }, PREFIX_DATA },
> + { VEX_W_TABLE (VEX_W_0F3850) },
> + { VEX_W_TABLE (VEX_W_0F3851) },
> { PREFIX_TABLE (PREFIX_EVEX_0F3852) },
> { PREFIX_TABLE (PREFIX_EVEX_0F3853) },
> { "vpopcnt%BW", { XM, EXx }, PREFIX_DATA },
> --- a/opcodes/i386-dis-evex-prefix.h
> +++ b/opcodes/i386-dis-evex-prefix.h
> @@ -233,14 +233,14 @@
> {
> { Bad_Opcode },
> { "vdpbf16p%XS", { XM, Vex, EXx }, 0 },
> - { "vpdpwssd", { XM, Vex, EXx }, 0 },
> + { VEX_W_TABLE (VEX_W_0F3852) },
> { "vp4dpwssd", { XM, Vex, EXxmm }, 0 },
> },
> /* PREFIX_EVEX_0F3853 */
> {
> { Bad_Opcode },
> { Bad_Opcode },
> - { "vpdpwssds", { XM, Vex, EXx }, 0 },
> + { VEX_W_TABLE (VEX_W_0F3853) },
> { "vp4dpwssds", { XM, Vex, EXxmm }, 0 },
> },
> /* PREFIX_EVEX_0F3868 */
OK.
On 14.10.2022 19:28, H.J. Lu wrote:
> On Fri, Oct 14, 2022 at 3:22 AM Jan Beulich <jbeulich@suse.com> wrote:
>>
>> Make %XV also print the separating blank in the VEX case, while making
>> it do nothing for EVEX-encoded insns. This way the AVX-VNNI entries
>> can be re-used for AVX512-VNNI, at the same time fixing the lack of
>> EVEX.W decoding.
>>
>> For the AVX-VNNI ones further make sure only VEX.66 forms are actually
>> decoded.
>> ---
>> Irrespective of this change I continue to disagree with the arbitrary
>> printing of "{vex}" for the AVX-VNNI insns: If that's meant for
>> disambiguation purposes, then EVEX-encoded insns not using EVEX-specific
>> functionality by having VEX counterparts (vaddps %xmm0, %xmm0, %xmm0)
>> should also be prefixed by "{evex}".
>
> This is done to match the assembler. There are 3 kinds of VNNI processors:
>
> 1. AVX512-VNNI only.
> 2. AVX-VNNI only.
> 3. AVX512-VNNI and AVX-VNNI.
>
> Since AVX512-VNNI came out first, all VNNI instructions without a prefix
> are encoded as AVX512-VNNI. The existing VNNI instructions without
> a prefix, generated by compiler or hand written, are encoded with EVEX.
> If one needs VNNI with VEX encoding, the {vex} prefix should be used.
... if, as said, AVX512 wasn't turned off altogether.
With your model, just look at how odd code using both AVX-VNNI and
AVX-VNNI-INT8 then looks:
vpdpbssd %ymm0, %ymm5, %ymm6
vpdpbsud %ymm1, %ymm5, %ymm6
{vex} vpdpbusd %ymm2, %ymm5, %ymm6
vpdpbuud %ymm3, %ymm5, %ymm6
Yes, one could further clutter this and add {vex} to every line. But
why would anyone want to clutter their code?
Plus the same argument then applies to AVX512VL: This came out later than
AVX, and the assembler (necessarily) requires {evex} to actually encode
these (when there are AVX equivalents). Hence to match the assembler, the
disassembler then also ought to emit {evex} for this subset of encodings.
Jan
> This applies to any AVX extensions which come after EVEX ones, including
> AVX-IFMA.
On Sun, Oct 16, 2022 at 11:39 PM Jan Beulich <jbeulich@suse.com> wrote:
>
> On 14.10.2022 19:28, H.J. Lu wrote:
> > On Fri, Oct 14, 2022 at 3:22 AM Jan Beulich <jbeulich@suse.com> wrote:
> >>
> >> Make %XV also print the separating blank in the VEX case, while making
> >> it do nothing for EVEX-encoded insns. This way the AVX-VNNI entries
> >> can be re-used for AVX512-VNNI, at the same time fixing the lack of
> >> EVEX.W decoding.
> >>
> >> For the AVX-VNNI ones further make sure only VEX.66 forms are actually
> >> decoded.
> >> ---
> >> Irrespective of this change I continue to disagree with the arbitrary
> >> printing of "{vex}" for the AVX-VNNI insns: If that's meant for
> >> disambiguation purposes, then EVEX-encoded insns not using EVEX-specific
> >> functionality by having VEX counterparts (vaddps %xmm0, %xmm0, %xmm0)
> >> should also be prefixed by "{evex}".
> >
> > This is done to match the assembler. There are 3 kinds of VNNI processors:
> >
> > 1. AVX512-VNNI only.
> > 2. AVX-VNNI only.
> > 3. AVX512-VNNI and AVX-VNNI.
> >
> > Since AVX512-VNNI came out first, all VNNI instructions without a prefix
> > are encoded as AVX512-VNNI. The existing VNNI instructions without
> > a prefix, generated by compiler or hand written, are encoded with EVEX.
> > If one needs VNNI with VEX encoding, the {vex} prefix should be used.
>
> ... if, as said, AVX512 wasn't turned off altogether.
>
> With your model, just look at how odd code using both AVX-VNNI and
> AVX-VNNI-INT8 then looks:
>
> vpdpbssd %ymm0, %ymm5, %ymm6
> vpdpbsud %ymm1, %ymm5, %ymm6
> {vex} vpdpbusd %ymm2, %ymm5, %ymm6
> vpdpbuud %ymm3, %ymm5, %ymm6
>
> Yes, one could further clutter this and add {vex} to every line. But
> why would anyone want to clutter their code?
This ensures that VNNI instructions without a prefix are encoded
with EVEX.
> Plus the same argument then applies to AVX512VL: This came out later than
> AVX, and the assembler (necessarily) requires {evex} to actually encode
> these (when there are AVX equivalents). Hence to match the assembler, the
> disassembler then also ought to emit {evex} for this subset of encodings.
A patch is welcome.
> Jan
>
> > This applies to any AVX extensions which come after EVEX ones, including
> > AVX-IFMA.
>
On 17.10.2022 23:10, H.J. Lu wrote:
> On Sun, Oct 16, 2022 at 11:39 PM Jan Beulich <jbeulich@suse.com> wrote:
>>
>> On 14.10.2022 19:28, H.J. Lu wrote:
>>> On Fri, Oct 14, 2022 at 3:22 AM Jan Beulich <jbeulich@suse.com> wrote:
>>>>
>>>> Make %XV also print the separating blank in the VEX case, while making
>>>> it do nothing for EVEX-encoded insns. This way the AVX-VNNI entries
>>>> can be re-used for AVX512-VNNI, at the same time fixing the lack of
>>>> EVEX.W decoding.
>>>>
>>>> For the AVX-VNNI ones further make sure only VEX.66 forms are actually
>>>> decoded.
>>>> ---
>>>> Irrespective of this change I continue to disagree with the arbitrary
>>>> printing of "{vex}" for the AVX-VNNI insns: If that's meant for
>>>> disambiguation purposes, then EVEX-encoded insns not using EVEX-specific
>>>> functionality by having VEX counterparts (vaddps %xmm0, %xmm0, %xmm0)
>>>> should also be prefixed by "{evex}".
>>>
>>> This is done to match the assembler. There are 3 kinds of VNNI processors:
>>>
>>> 1. AVX512-VNNI only.
>>> 2. AVX-VNNI only.
>>> 3. AVX512-VNNI and AVX-VNNI.
>>>
>>> Since AVX512-VNNI came out first, all VNNI instructions without a prefix
>>> are encoded as AVX512-VNNI. The existing VNNI instructions without
>>> a prefix, generated by compiler or hand written, are encoded with EVEX.
>>> If one needs VNNI with VEX encoding, the {vex} prefix should be used.
>>
>> ... if, as said, AVX512 wasn't turned off altogether.
>>
>> With your model, just look at how odd code using both AVX-VNNI and
>> AVX-VNNI-INT8 then looks:
>>
>> vpdpbssd %ymm0, %ymm5, %ymm6
>> vpdpbsud %ymm1, %ymm5, %ymm6
>> {vex} vpdpbusd %ymm2, %ymm5, %ymm6
>> vpdpbuud %ymm3, %ymm5, %ymm6
>>
>> Yes, one could further clutter this and add {vex} to every line. But
>> why would anyone want to clutter their code?
>
> This ensures that VNNI instructions without a prefix are encoded
> with EVEX.
As does ".arch .noavx512f".
>> Plus the same argument then applies to AVX512VL: This came out later than
>> AVX, and the assembler (necessarily) requires {evex} to actually encode
>> these (when there are AVX equivalents). Hence to match the assembler, the
>> disassembler then also ought to emit {evex} for this subset of encodings.
>
> A patch is welcome.
Will add to my list. That'll be a huge patch, I guess, because of the
sometimes excessive testcases that were added for the various sub-features.
Jan
@@ -1755,7 +1755,7 @@ struct dis386 {
"XD" => print 'd' if !EVEX or EVEX.W=1, EVEX.W=0 is not a valid encoding
"XH" => print 'h' if EVEX.W=0, EVEX.W=1 is not a valid encoding (for FP16)
"XS" => print 's' if !EVEX or EVEX.W=0, EVEX.W=1 is not a valid encoding
- "XV" => print "{vex3}" pseudo prefix
+ "XV" => print "{vex} " pseudo prefix
"LQ" => print 'l' ('d' in Intel mode) or 'q' for memory operand, cond
being false, or no operand at all in 64bit mode, or if suffix_always
is true.
@@ -7545,19 +7545,19 @@ static const struct dis386 vex_w_table[]
},
{
/* VEX_W_0F3850 */
- { "%XV vpdpbusd", { XM, Vex, EXx }, 0 },
+ { "%XVvpdpbusd", { XM, Vex, EXx }, PREFIX_DATA },
},
{
/* VEX_W_0F3851 */
- { "%XV vpdpbusds", { XM, Vex, EXx }, 0 },
+ { "%XVvpdpbusds", { XM, Vex, EXx }, PREFIX_DATA },
},
{
/* VEX_W_0F3852 */
- { "%XV vpdpwssd", { XM, Vex, EXx }, 0 },
+ { "%XVvpdpwssd", { XM, Vex, EXx }, PREFIX_DATA },
},
{
/* VEX_W_0F3853 */
- { "%XV vpdpwssds", { XM, Vex, EXx }, 0 },
+ { "%XVvpdpwssds", { XM, Vex, EXx }, PREFIX_DATA },
},
{
/* VEX_W_0F3858 */
@@ -10711,22 +10711,29 @@ putop (instr_info *ins, const char *in_t
case 'V':
if (l == 0)
abort ();
- else if (l == 1
- && (last[0] == 'L' || last[0] == 'X'))
+ else if (l == 1)
{
- if (last[0] == 'X')
+ switch (last[0])
{
+ case 'X':
+ if (ins->vex.evex)
+ break;
*ins->obufp++ = '{';
*ins->obufp++ = 'v';
*ins->obufp++ = 'e';
*ins->obufp++ = 'x';
*ins->obufp++ = '}';
- }
- else if (ins->rex & REX_W)
- {
+ *ins->obufp++ = ' ';
+ break;
+ case 'L':
+ if (!(ins->rex & REX_W))
+ break;
*ins->obufp++ = 'a';
*ins->obufp++ = 'b';
*ins->obufp++ = 's';
+ break;
+ default:
+ abort ();
}
}
else
@@ -383,8 +383,8 @@ static const struct dis386 evex_table[][
{ "vrsqrt14p%XW", { XM, EXx }, 0 },
{ "vrsqrt14s%XW", { XMScalar, VexScalar, EXdq }, PREFIX_DATA },
/* 50 */
- { "vpdpbusd", { XM, Vex, EXx }, PREFIX_DATA },
- { "vpdpbusds", { XM, Vex, EXx }, PREFIX_DATA },
+ { VEX_W_TABLE (VEX_W_0F3850) },
+ { VEX_W_TABLE (VEX_W_0F3851) },
{ PREFIX_TABLE (PREFIX_EVEX_0F3852) },
{ PREFIX_TABLE (PREFIX_EVEX_0F3853) },
{ "vpopcnt%BW", { XM, EXx }, PREFIX_DATA },
@@ -233,14 +233,14 @@
{
{ Bad_Opcode },
{ "vdpbf16p%XS", { XM, Vex, EXx }, 0 },
- { "vpdpwssd", { XM, Vex, EXx }, 0 },
+ { VEX_W_TABLE (VEX_W_0F3852) },
{ "vp4dpwssd", { XM, Vex, EXxmm }, 0 },
},
/* PREFIX_EVEX_0F3853 */
{
{ Bad_Opcode },
{ Bad_Opcode },
- { "vpdpwssds", { XM, Vex, EXx }, 0 },
+ { VEX_W_TABLE (VEX_W_0F3853) },
{ "vp4dpwssds", { XM, Vex, EXxmm }, 0 },
},
/* PREFIX_EVEX_0F3868 */