public inbox for binutils@sourceware.org
 help / color / mirror / Atom feed
From: Jan Beulich <jbeulich@suse.com>
To: "Jiang, Haochen" <haochen.jiang@intel.com>
Cc: "hjl.tools@gmail.com" <hjl.tools@gmail.com>,
	"binutils@sourceware.org" <binutils@sourceware.org>
Subject: Re: [PATCH v3] Support ymm rounding control for Intel AVX10.2
Date: Fri, 16 Aug 2024 08:19:18 +0200	[thread overview]
Message-ID: <97b022db-6da8-418a-9a3a-ae763b5ca2ce@suse.com> (raw)
In-Reply-To: <SA1PR11MB5946731D099FA409A17B1811EC812@SA1PR11MB5946.namprd11.prod.outlook.com>

On 16.08.2024 04:14, Jiang, Haochen wrote:
>> -----Original Message-----
>> From: Jan Beulich <jbeulich@suse.com>
>> Sent: Thursday, August 15, 2024 11:30 PM
>>
>> On 15.08.2024 15:08, Jiang, Haochen wrote:
>>>> -----Original Message-----
>>>> From: Jan Beulich <jbeulich@suse.com>
>>>> Sent: Thursday, August 15, 2024 5:24 PM
>>>>
>>>> On 15.08.2024 03:06, Jiang, Haochen wrote:
>>>>>>> --- a/opcodes/i386-opc.tbl
>>>>>>> +++ b/opcodes/i386-opc.tbl
>>>>>>> @@ -156,6 +156,8 @@
>>>>>>>  // substantially similar), depending on what encoding was requested.
>>>>>>>  #define APX_F(cpuid) cpuid&(cpuid|APX_F)
>>>>>>>
>>>>>>> +#define AVX10_2(cpuid) cpuid&(cpuid|AVX10_2)
>>>>>>
>>>>>> ... this. The StaticRounding / SAE insn forms are all suitably
>>>>>> identified by these two attributes. So far they applied to EVEX512
>>>>>> only. All that changes is that now they apply to EVEX256 as well.
>>>>>> As long as AVX10.2 is available, of course. Therefore all (or at
>>>>>> least
>>>>>> most) of the templates should be possible to leave alone (on the
>>>>>> assumption that there are no outliers, i.e. no mnemonics which
>>>>>> allow RC/SAE in 512-bit forms bit not in 256-bit ones). You already
>>>>>> change check_VecOperands(), it just needs doing a little
>>>>>> differently (in particular without setting .ymm and without using
>> maybe_cpu()).
>>>>>>
>>>>>> To achieve this, it may end up necessary to split encoding_evex512
>>>>>> into two enumerators, the new one being encoding_sae.
>>>>>
>>>>> I haven't investigated that fully, but I suppose we can definitely do that.
>>>>> The concern is if we should do that.
>>>>>
>>>>> IMO,  if doing that and not adding AVX10.2 to table, it means that
>>>>> the ymm rounding in AVX10.2 is implicitly enabled, unlike all the
>>>>> other features which are explicitly enabled. It will cause confusion
>>>>> when someone wants to go through the table to know which feature
>>>>> belongs to which ISA. It will also take more time to investigate
>>>>> that why suddenly ymm rounding is turned on if they are not familiar with the
>> latest ISA.
>>>>
>>>> The table's primary purpose isn't documentation. One of the goals
>>>> continues to be to limit the number of templates we have, to in turn
>>>> limit the number of matching attempts that need doing while trying to
>>>> find the template for a given insn. That's
>>>
>>> I get your concern. However, my change is only altering the CPUIDs. No
>>> new templates is added in the table. Therefore, it won't add the
>>> number of templates for going through the whole table of templates.
>>>
>>> For the effort side, whatever way we are using, we will always need to
>>> find the original AVX512 template, which is also the template I
>>> changed. It is exactly the same path per my understanding.
>>>
>>> If that is the only concern, it is somehow not that convinced for me.
>>
>> Hmm, you're right, I was still having v1 in mind, it seems. Nevertheless:
>> Why a more complicated (and hence harder to parse/understand) CPU specifier
>> than needed? If the same functionality can be achieved while leaving the
>> templates alone, I guess I'd still prefer that.
> 
> I would be surprised if the current CPUID change would be more complicated
> than changing encoding_evex512. CPUID change looks straightforward since
> it explicitly mentioned AVX10.2.

Well. In the change here I'm having difficulty seeing that the sole use of
maybe_cpu (t, CpuAVX10_2) is actually required. It looks to me as if, with
it dropped, things would still work. If that was the case, the entire
opcode table adjustment would be purely (as you put it earlier) for doc
purposes. Instead I think that irrespective of the opcode table changes
you're making, the splitting of the encoding enumerator may still be
needed; or, the other way around, we may get away without the split either
way. However, even if the split wasn't strictly needed, it may still be
advisable to do, to avoid the present naming ending up as misleading. But
such tidying can also be done subsequently, and it doesn't need to be you
to do it (especially if you would rather not want to do that).

Jan

  reply	other threads:[~2024-08-16  6:19 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-08-09  1:57 Haochen Jiang
2024-08-12 11:53 ` Jan Beulich
2024-08-15  1:06   ` Jiang, Haochen
2024-08-15  9:23     ` Jan Beulich
2024-08-15 13:08       ` Jiang, Haochen
2024-08-15 15:29         ` Jan Beulich
2024-08-16  2:14           ` Jiang, Haochen
2024-08-16  6:19             ` Jan Beulich [this message]
2024-08-16  6:26               ` Jiang, Haochen

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=97b022db-6da8-418a-9a3a-ae763b5ca2ce@suse.com \
    --to=jbeulich@suse.com \
    --cc=binutils@sourceware.org \
    --cc=haochen.jiang@intel.com \
    --cc=hjl.tools@gmail.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).