[PATCH 04/10] Support Intel CMPccXADD

[Jan Beulich]

On 26.10.2022 05:03, Jiang, Haochen wrote:
>> -----Original Message-----
>> From: Jan Beulich <jbeulich@suse.com>
>> Sent: Tuesday, October 25, 2022 2:53 PM
>> To: Jiang, Haochen <haochen.jiang@intel.com>
>> Cc: hjl.tools@gmail.com; binutils@sourceware.org
>> Subject: Re: [PATCH 04/10] Support Intel CMPccXADD
>>
>> On 24.10.2022 07:55, Jiang, Haochen wrote:
>>>> -----Original Message-----
>>>> From: Jan Beulich <jbeulich@suse.com>
>>>> Sent: Friday, October 14, 2022 9:47 PM
>>>>
>>>> On 14.10.2022 11:12, Haochen Jiang wrote:
>>>> And finally - is SwapSources really appropriate to use here? There's only
>> one
>>>> pure source operand, the other two are also serving as destinations.
>>>> I wonder whether an attribute is necessary here in the first place: Vex-
>> encoded
>>>> insns with a memory destination never have two further register
>> operands, so
>>>> that property should suffice for identifying the case in
>> build_modrm_byte().
>>>> Alternatively you could also simply use the CPU flag.
>>>
>>> We may need a special identifier for CMPccXADD since we have VVVV at
>>> operand 3, where it is always at operand 2 for all other insts which
>>> have VVVV. That is the reason we reuse SwapSources. It might be not
>>> that same as the original meaning. But we want to avoid adding a bit
>>> for this very rare case. Do we need to change that?
>>
>> Re-using existing attributes is certainly preferred. But the question here
>> was whether _any_ special attribute is needed. Did you try out my
>> suggestion,
>> and it didn't work out for some reason? Avoiding the (ab)use of an
>> inappropriately (for the purpose here) named attribute would imo be
>> preferable.
> 
> Actually we have some similar instructions. For example, vmaskmovps/d.
> It could also take one memory operand as dest and two register operands
> as source.

As you say there are two _source_ operands here, whereas ...

> However, cmp<cc>xadd has a different encoding pattern with it. The default
> behavior for previous insts like vmaskmovps/d in Intel syntax is to encode
> first register operand as vvvv and second register operand as modrm:reg.
> In cmp<cc>xadd, it is swapped, with first register operand as modrm:reg and
> second register operand as vvvv. I suppose it makes sense to use SwapSource
> and it is quite hard or might be dirty to just use the number of registers to
> identify them in build_modrm_byte().

... this has two destinations (memory and a register). Which is why using
SwapSource at best is a slight abuse (which, as said, I'd prefer to avoid).
But yes, I agree that going _just_ from a memory destination with two
further register ops is not sufficient.

Seeing what H.J. and I are suggesting for the PREFETCHI change: How about
using the CPU flag in the conditional that you add?

Jan