RE: [PATCH 04/10] Support Intel CMPccXADD

["Jiang, Haochen" <haochen.jiang@intel.com>]

> -----Original Message-----
> From: Jan Beulich <jbeulich@suse.com>
> Sent: Wednesday, October 26, 2022 4:50 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 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?

That is ok for me. I suppose the cmpccxadd might be the special one for very
long time. 

However, the potential problem is when the same scenario happen again. We
will have to add another cpuid at that condition again, which is a little dirty.

Haochen

> 
> Jan