From: Jan Beulich <jbeulich@suse.com>
To: "Cui, Lili" <lili.cui@intel.com>
Cc: "hjl.tools@gmail.com" <hjl.tools@gmail.com>,
"binutils@sourceware.org" <binutils@sourceware.org>
Subject: Re: [PATCH V2] Support APX NF
Date: Mon, 18 Mar 2024 12:50:46 +0100 [thread overview]
Message-ID: <947a7263-8075-41ed-a842-d620762d3fbe@suse.com> (raw)
In-Reply-To: <SJ0PR11MB5600354D7CC14EA25D9C3FF09E2D2@SJ0PR11MB5600.namprd11.prod.outlook.com>
On 18.03.2024 12:21, Cui, Lili wrote:
>> On 13.03.2024 03:54, Cui, Lili wrote:
>>>> On 12.03.2024 14:22, Cui, Lili wrote:
>>>>> // Arithmetic.
>>>>> add, 0x0, APX_F,
>>>>> D|C|W|CheckOperandSize|Modrm|No_sSuf|DstVVVV|EVexMap4|NF, {
>>>>> Reg8|Reg16|Reg32|Reg64,
>>>> Reg8|Reg16|Reg32|Reg64|Unspecified|BaseIndex,
>>>>> Reg8|Reg16|Reg32|Reg64 } -add, 0x0, 0,
>>>>> D|W|CheckOperandSize|Modrm|No_sSuf|HLEPrefixLock, {
>>>>> Reg8|Reg16|Reg32|Reg64,
>>>> Reg8|Reg16|Reg32|Reg64|Unspecified|BaseIndex }
>>>>> -add, 0x0, APX_F,
>>>> D|W|CheckOperandSize|Modrm|No_sSuf|EVexMap4|NF, {
>>>>> Reg8|Reg16|Reg32|Reg64,
>>>> Reg8|Reg16|Reg32|Reg64|Unspecified|BaseIndex }
>>>>> +add<cpu1>, 0x0, 0,
>>>> D|W|CheckOperandSize|Modrm|No_sSuf|<cpu1:attr>, {
>>>>> +Reg8|Reg16|Reg32|Reg64,
>>>> Reg8|Reg16|Reg32|Reg64|Unspecified|BaseIndex
>>>>> +}
>>>>
>>>> Differentiating on other than the mnemonic is possible (see e.g. the
>>>> mmx and sse templates, which append nothing to the mnemonics) but not
>>>> wanted here. The goal of the suggested template is to have something
>>>> that covers all 7 mnemonics in one go, including their legacy forms.
>>>>
>>>
>>> The handling of <mmx/<sse are clever. The current templates are indeed
>> redundant. Do you mind I create a separate patch to clean up all APX
>> templates instead of in this patch.
>>
>> Well - it explicitly ought to be a separate patch. Yet FTAOD - afaict such a new
>> (set of) template(s) should not follow mmx/sse, but rather e.g. fma or
>> pseudopfx (covering the entire mnemonic then).
>>
>
> Hi Jan,
>
> Below are two examples of merging, does it meet expectations?
>
> --- a/opcodes/i386-gen.c
> +++ b/opcodes/i386-gen.c
>
> @@ -1904,11 +1904,15 @@ process_i386_opcodes (FILE *table)
> case '<':
> parse_template (p, lineno);
> continue;
> + case '@':
> + p++;
> + break;
> default:
> if (!marker)
> continue;
> break;
> }
>
> rol and ror can be merged.
>
> -rol, 0xd0/0, APX_F, W|Modrm|No_sSuf|CheckOperandSize|DstVVVV|EVexMap4|NF, { Imm1, Reg8|Reg16|Reg32|Reg64|Unspecified|BaseIndex, Reg8|Reg16|Reg32|Reg64 }
> -rol, 0xd0/0, 0, W|Modrm|No_sSuf, { Imm1, Reg8|Reg16|Reg32|Reg64|Unspecified|BaseIndex }
> -rol, 0xd0/0, APX_F, W|Modrm|No_sSuf|EVexMap4|NF, { Imm1, Reg8|Reg16|Reg32|Reg64|Unspecified|BaseIndex }
> -rol, 0xc0/0, APX_F, W|Modrm|No_sSuf|CheckOperandSize|DstVVVV|EVexMap4|NF, { Imm8|Imm8S, Reg8|Reg16|Reg32|Reg64|Unspecified|BaseIndex, Reg8|Reg16|Reg32|Reg64 }
> -rol, 0xc0/0, i186, W|Modrm|No_sSuf, { Imm8|Imm8S, Reg8|Reg16|Reg32|Reg64|Unspecified|BaseIndex }
> -rol, 0xc0/0, APX_F, W|Modrm|No_sSuf|EVexMap4|NF, { Imm8|Imm8S, Reg8|Reg16|Reg32|Reg64|Unspecified|BaseIndex }
> -rol, 0xd2/0, APX_F, W|Modrm|No_sSuf|CheckOperandSize|DstVVVV|EVexMap4|NF, { ShiftCount, Reg8|Reg16|Reg32|Reg64|Unspecified|BaseIndex, Reg8|Reg16|Reg32|Reg64 }
> -rol, 0xd2/0, 0, W|Modrm|No_sSuf, { ShiftCount, Reg8|Reg16|Reg32|Reg64|Unspecified|BaseIndex }
> -rol, 0xd2/0, APX_F, W|Modrm|No_sSuf|EVexMap4|NF, { ShiftCount, Reg8|Reg16|Reg32|Reg64|Unspecified|BaseIndex }
> -rol, 0xd0/0, 0, W|Modrm|No_sSuf, { Reg8|Reg16|Reg32|Reg64|Unspecified|BaseIndex }
> -rol, 0xd0/0, APX_F, W|Modrm|No_sSuf|EVexMap4|NF, { Reg8|Reg16|Reg32|Reg64|Unspecified|BaseIndex }
> -
> -ror, 0xd0/1, APX_F, W|Modrm|No_sSuf|CheckOperandSize|DstVVVV|EVexMap4|NF, { Imm1, Reg8|Reg16|Reg32|Reg64|Unspecified|BaseIndex, Reg8|Reg16|Reg32|Reg64 }
> -ror, 0xd0/1, 0, W|Modrm|No_sSuf, { Imm1, Reg8|Reg16|Reg32|Reg64|Unspecified|BaseIndex }
> -ror, 0xd0/1, APX_F, W|Modrm|No_sSuf|EVexMap4|NF, { Imm1, Reg8|Reg16|Reg32|Reg64|Unspecified|BaseIndex }
> -ror, 0xc0/1, APX_F, W|Modrm|No_sSuf|CheckOperandSize|DstVVVV|EVexMap4|NF, { Imm8|Imm8S, Reg8|Reg16|Reg32|Reg64|Unspecified|BaseIndex, Reg8|Reg16|Reg32|Reg64 }
> -ror, 0xc0/1, i186, W|Modrm|No_sSuf, { Imm8|Imm8S, Reg8|Reg16|Reg32|Reg64|Unspecified|BaseIndex }
> -ror, 0xc0/1, APX_F, W|Modrm|No_sSuf|EVexMap4|NF, { Imm8|Imm8S, Reg8|Reg16|Reg32|Reg64|Unspecified|BaseIndex }
> -ror, 0xd2/1, APX_F, W|Modrm|No_sSuf|CheckOperandSize|DstVVVV|EVexMap4|NF, { ShiftCount, Reg8|Reg16|Reg32|Reg64|Unspecified|BaseIndex, Reg8|Reg16|Reg32|Reg64 }
> -ror, 0xd2/1, 0, W|Modrm|No_sSuf, { ShiftCount, Reg8|Reg16|Reg32|Reg64|Unspecified|BaseIndex }
> -ror, 0xd2/1, APX_F, W|Modrm|No_sSuf|EVexMap4|NF, { ShiftCount, Reg8|Reg16|Reg32|Reg64|Unspecified|BaseIndex }
> -ror, 0xd0/1, 0, W|Modrm|No_sSuf, { Reg8|Reg16|Reg32|Reg64|Unspecified|BaseIndex }
> -ror, 0xd0/1, APX_F, W|Modrm|No_sSuf|EVexMap4|NF, { Reg8|Reg16|Reg32|Reg64|Unspecified|BaseIndex }
> +<extend10:cpu:attr:reg, +
> + $ndd:APX_F:CheckOperandSize|DstVVVV|EVexMap4|NF:Reg8|Reg16|Reg32|Reg64, +
> + $legacy:0::, +
> + $nf:APX_F:EVexMap4|NF:>
> +
> +<extend11:cpu:attr:reg, +
> + $ndd:APX_F:CheckOperandSize|DstVVVV|EVexMap4|NF:Reg8|Reg16|Reg32|Reg64, +
> + $legacy:i186::, +
> + $nf:APX_F:EVexMap4|NF:>
> +
> +<extend12:cpu:attr, +
> + $legacy:0:, +
> + $nf:APX_F:EVexMap4|NF>
I don't understand these three. I'd have expected all attributes to be
specified ...
> +<mn:opcode, rol:0, ror:1>
... here. Same then for add, where I'd have expected a similar template as the
one above, e.g.
<alu:opcode, add:0, sub:...>
(but of course with more than just "opcode" specified).
> +@<mn><extend10>, 0xd0/0|<mn:opcode>, <extend10:cpu>, W|Modrm|No_sSuf|<extend10:attr>, { Imm1, Reg8|Reg16|Reg32|Reg64|Unspecified|BaseIndex, <extend10:reg> }
> +
> +@<mn><extend11>, 0xc0/0|<mn:opcode>, <extend11:cpu>, W|Modrm|No_sSuf|<extend11:attr>, { Imm8|Imm8S, Reg8|Reg16|Reg32|Reg64|Unspecified|BaseIndex, <extend11:reg> }
> +
> +@<mn><extend10>, 0xd2/0|<mn:opcode>, <extend10:cpu>, W|Modrm|No_sSuf|<extend10:attr>, { ShiftCount, Reg8|Reg16|Reg32|Reg64|Unspecified|BaseIndex, <extend10:reg> }
> +
> +@<mn><extend12>, 0xd0/0|<mn:opcode>, <extend12:cpu>, W|Modrm|No_sSuf|<extend12:attr>, { Reg8|Reg16|Reg32|Reg64|Unspecified|BaseIndex }
> +
> ...
>
> The same instruction as add structure was not found. There are always some subtle differences.
I expect the issue is with you using just a single "attr", trying to put all
attributes in there. When split (see e.g. <sdh>), folding ought to be possible.
I'd also prefer if we could avoid the need for the leading @.
Anyway - feel free to leave this to me. While not overly nice, I guess we can
live with the growing volume due to what NF wants to add.
Jan
> +<extend1:cpu:attr:reg, +
> + $ndd:APX_F:C|DstVVVV|EVexMap4|NF:Reg8|Reg16|Reg32|Reg64, +
> + $legacy:0:HLEPrefixLock:, +
> + $nf:APX_F:EVexMap4|NF:>
> +
> +<extend2:cpu:attr:reg, +
> + $ndd:APX_F:CheckOperandSize|DstVVVV|EVexMap4|NF:Reg16|Reg32|Reg64, +
> + $legacy:0:HLEPrefixLock:, +
> + $nf:APX_F:EVexMap4|NF:>
> +
> +<extend3:cpu:attr:reg, +
> + $ndd:APX_F:CheckOperandSize|DstVVVV|EVexMap4|NF:Reg8|Reg16|Reg32|Reg64, +
> + $legacy:0:HLEPrefixLock:, +
> + $nf:APX_F:EVexMap4|NF:>
> +
> -add, 0x0, APX_F, D|C|W|CheckOperandSize|Modrm|No_sSuf|DstVVVV|EVexMap4|NF, { Reg8|Reg16|Reg32|Reg64, Reg8|Reg16|Reg32|Reg64|Unspecified|BaseIndex, Reg8|Reg16|Reg32|Reg64 }
> -add, 0x0, 0, D|W|CheckOperandSize|Modrm|No_sSuf|HLEPrefixLock, { Reg8|Reg16|Reg32|Reg64, Reg8|Reg16|Reg32|Reg64|Unspecified|BaseIndex }
> -add, 0x0, APX_F, D|W|CheckOperandSize|Modrm|No_sSuf|EVexMap4|NF, { Reg8|Reg16|Reg32|Reg64, Reg8|Reg16|Reg32|Reg64|Unspecified|BaseIndex }
> -add, 0x83/0, APX_F, Modrm|CheckOperandSize|No_bSuf|No_sSuf|DstVVVV|EVexMap4|NF, { Imm8S, Reg16|Reg32|Reg64|Unspecified|BaseIndex, Reg16|Reg32|Reg64 }
> -add, 0x83/0, 0, Modrm|No_bSuf|No_sSuf|HLEPrefixLock, { Imm8S, Reg16|Reg32|Reg64|Unspecified|BaseIndex }
> -add, 0x83/0, APX_F, Modrm|No_bSuf|No_sSuf|EVexMap4|NF, { Imm8S, Reg16|Reg32|Reg64|Unspecified|BaseIndex }
> +add<extend1>, 0x0, <extend1:cpu>, D|W|CheckOperandSize|Modrm|No_sSuf|<extend1:attr>, { Reg8|Reg16|Reg32|Reg64, Reg8|Reg16|Reg32|Reg64|Unspecified|BaseIndex, <extend1:reg> }
> +add<extend2>, 0x83/0, <extend2:cpu>, Modrm|No_bSuf|No_sSuf|<extend2:attr>, { Imm8S, Reg16|Reg32|Reg64|Unspecified|BaseIndex, <extend2:reg> }
> add, 0x4, 0, W|No_sSuf, { Imm8|Imm16|Imm32|Imm32S, Acc|Byte|Word|Dword|Qword }
> -add, 0x80/0, APX_F, W|Modrm|CheckOperandSize|No_sSuf|DstVVVV|EVexMap4|NF, { Imm8|Imm16|Imm32|Imm32S, Reg8|Reg16|Reg32|Reg64|Unspecified|BaseIndex, Reg8|Reg16|Reg32|Reg64}
> -add, 0x80/0, 0, W|Modrm|No_sSuf|HLEPrefixLock, { Imm8|Imm16|Imm32|Imm32S, Reg8|Reg16|Reg32|Reg64|Unspecified|BaseIndex }
> -add, 0x80/0, APX_F, W|Modrm|No_sSuf|EVexMap4|NF, { Imm8|Imm16|Imm32|Imm32S, Reg8|Reg16|Reg32|Reg64|Unspecified|BaseIndex }
> +add<extend3>, 0x80/0, <extend3:cpu>, W|Modrm|No_sSuf|<extend3:attr>, { Imm8|Imm16|Imm32|Imm32S, Reg8|Reg16|Reg32|Reg64|Unspecified|BaseIndex, <extend3:reg> }
>
>
> Thanks,
> Lili.
next prev parent reply other threads:[~2024-03-18 11:50 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-03-04 8:15 Cui, Lili
2024-03-08 9:36 ` Jan Beulich
2024-03-11 13:54 ` Cui, Lili
2024-03-11 14:09 ` Jan Beulich
2024-03-12 6:12 ` Cui, Lili
2024-03-12 7:46 ` Jan Beulich
2024-03-12 8:51 ` Cui, Lili
2024-03-12 13:22 ` Cui, Lili
2024-03-12 13:53 ` Jan Beulich
2024-03-13 2:54 ` Cui, Lili
2024-03-13 7:36 ` Jan Beulich
2024-03-18 11:21 ` Cui, Lili
2024-03-18 11:50 ` Jan Beulich [this message]
2024-03-18 13:43 ` Cui, Lili
2024-03-19 1:24 ` Cui, Lili
2024-03-08 10:40 ` Jan Beulich
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=947a7263-8075-41ed-a842-d620762d3fbe@suse.com \
--to=jbeulich@suse.com \
--cc=binutils@sourceware.org \
--cc=hjl.tools@gmail.com \
--cc=lili.cui@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).