[PATCH 06/10] Support Intel RAO-INT

[Jan Beulich]

On 28.10.2022 10:10, Jiang, Haochen wrote:
>> -----Original Message-----
>> From: Jan Beulich <jbeulich@suse.com>
>> Sent: Friday, October 28, 2022 2:53 PM
>> To: H.J. Lu <hjl.tools@gmail.com>
>> Cc: Jiang, Haochen <haochen.jiang@intel.com>; binutils@sourceware.org;
>> Kong, Lingling <lingling.kong@intel.com>
>> Subject: Re: [PATCH 06/10] Support Intel RAO-INT
>>
>> On 27.10.2022 20:46, H.J. Lu wrote:
>>> On Wed, Oct 26, 2022 at 11:39 PM Jan Beulich <jbeulich@suse.com> wrote:
>>>>
>>>> On 27.10.2022 05:50, Jiang, Haochen wrote:
>>>>>> -----Original Message-----
>>>>>> From: Jan Beulich <jbeulich@suse.com>
>>>>>> Sent: Wednesday, October 26, 2022 4:57 PM
>>>>>> To: Jiang, Haochen <haochen.jiang@intel.com>
>>>>>> Cc: hjl.tools@gmail.com; binutils@sourceware.org; Kong, Lingling
>>>>>> <lingling.kong@intel.com>
>>>>>> Subject: Re: [PATCH 06/10] Support Intel RAO-INT
>>>>>>
>>>>>> On 26.10.2022 07:16, Jiang, Haochen wrote:
>>>>>>>> -----Original Message-----
>>>>>>>> From: Jan Beulich <jbeulich@suse.com>
>>>>>>>> Sent: Tuesday, October 25, 2022 3:01 PM
>>>>>>>>
>>>>>>>> On 24.10.2022 07:56, Jiang, Haochen wrote:
>>>>>>>>>> -----Original Message-----
>>>>>>>>>> From: Jan Beulich <jbeulich@suse.com>
>>>>>>>>>> Sent: Friday, October 14, 2022 10:38 PM
>>>>>>>>>>
>>>>>>>>>> On 14.10.2022 11:12, Haochen Jiang wrote:
>>>>>>>>>>> +aadd, 0xf38fc, None, CpuRAOINT,
>>>>>>>>>>>
>>>>>>>>
>>>>>>
>> +Modrm|IgnoreSize|No_bSuf|No_wSuf|No_sSuf|No_ldSuf,{ Reg32|Reg64,
>>>>>>>>>>> +Dword|Qword|Unspecified|BaseIndex}
>>>>>>>>>>> +aand, 0x660f38fc, None, CpuRAOINT,
>>>>>>>>>>>
>>>>>>>>
>>>>>>
>> +Modrm|IgnoreSize|No_bSuf|No_wSuf|No_sSuf|No_ldSuf,{ Reg32|Reg64,
>>>>>>>>>>> +Dword|Qword|Unspecified|BaseIndex}
>>>>>>>>>>> +aor, 0xf20f38fc, None, CpuRAOINT,
>>>>>>>>>>>
>>>>>>>>
>>>>>>
>> +Modrm|IgnoreSize|No_bSuf|No_wSuf|No_sSuf|No_ldSuf,{ Reg32|Reg64,
>>>>>>>>>>> +Dword|Qword|Unspecified|BaseIndex}
>>>>>>>>>>> +axor, 0xf30f38fc, None, CpuRAOINT,
>>>>>>>>>>>
>>>>>>>>
>>>>>>
>> +Modrm|IgnoreSize|No_bSuf|No_wSuf|No_sSuf|No_ldSuf,{ Reg32|Reg64,
>>>>>>>>>>> +Dword|Qword|Unspecified|BaseIndex}
>>>>>>>>>>
>>>>>>>>>> Why IgnoreSize? Instead I think you need CheckRegSize
>> (assuming
>>>>>>>>>> it
>>>>>> does
>>>>>>>>>> enough for Intel syntax memory operands - please double check;
>>>>>>>>>> if not
>>>>>>>> this will
>>>>>>>>>> need fixing).
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>> For table, we aligned with CMPccXADD and added No_lSuf and
>>>>>>>>> No_qSuf
>>>>>>>> since
>>>>>>>>> the suffixes are not required.
>>>>>>>>
>>>>>>>> Which, as said, I consider wrong both here and there.
>>>>>>>
>>>>>>> I still agree with H.J.. Also I suppose the previous usage of
>>>>>>> suffixes are quite messy and we should not stick on the confusing
>> code.
>>>>>>>
>>>>>>> Keeping consistent with SPEC from now on to avoid confusion on
>>>>>>> that
>>>>>> might
>>>>>>> be a better solution for future.
>>>>>>
>>>>>> Same question to you then: Which spec? As said in reply to H.J.,
>>>>>> the only spec I'm aware of suggests prefixes ought to be used
>>>>>> _whenever_ there's a sized memory operand [1]. If we want to deal
>>>>>> with the present "mess", I think we ought to simply allow suffixes
>>>>>> consistently. Please point out any other messy aspects beyond that.
>>>>>
>>>>> The default behavior for current ISA is when there is no suffix
>>>>> required, it won't mention that.
>>>>
>>>> What is "current ISA" here? Looking over the whole set of insns which
>>>> we support, it's the vast majority which behaves differently from
>>>> what you say. It's been several times that I mentioned that we would
>>>> better eliminate this inconsistency.
>>>>
>>>
>>> There is no ambiguity in the RAO-INT spec which requires a suffix.
>>> No suffix in A&T syntax is different from legacy instructions.  But it
>>> is consistent with the RAO-INT spec.
>>
>> The RAO-INT spec doesn't even know of AT&T syntax, so can't be taken as a
>> reference when deciding what the behavior there should be.
> 
> For instruction xadd, which is quite similar with RAO-INT, when we are using
> AT&T syntax, we also do not have suffix.]

Where do you take that from? The present table entry is

xadd, 0xfc0, None, Cpu486, W|CheckRegSize|Modrm|No_sSuf|No_ldSuf|HLEPrefixLock, { Reg8|Reg16|Reg32|Reg64, Reg8|Reg16|Reg32|Reg64|Byte|Word|Dword|Qword|Unspecified|BaseIndex }

which demonstrates that b, w, l, and q suffixes are all permitted.

> I suppose if that is not required, we do not need that since we can also know
> them from the register operand size.

Sure, the suffix can be derived from the involved registers. Therefore
on such insns it isn't required. But its use still ought to be permitted.

> BTW, should the suffix instruction dependent? It might be more operand
> related from my opinion. If that is the truth, could we just judge whether we
> should add them when dealing with memory operands?

I'm afraid I don't really understand what you're saying/asking here.
In any event - whether a suffix is required indeed depends on insn
operands. Yet even insns with (only) GPR operands _may_ use a suffix
in AT&T mode, irrespective of it being derivable from those GPR
operands. We actually apply consistency checks between registers used
and the suffix (if present).

Jan