[PATCH 04/10] Support Intel CMPccXADD

[Jiang, Haochen]

> -----Original Message-----
> From: Jan Beulich <jbeulich@suse.com>
> Sent: Thursday, October 27, 2022 2:38 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 27.10.2022 05:09, Jiang, Haochen wrote:
> >
> >
> >> -----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.
> 
> I guess we can decide at that point whether a 2nd CPU check is preferable
> over whatever alternative.

That sounds reasonable. Thanks for the whole discussion.

Haochen

> 
> Jan