[PATCH] x86: Add {load}/{store} tests for apx instructions.

[Cui, Lili]

> On 02.07.2024 15:49, Cui, Lili wrote:
> >> On 02.07.2024 10:22, Jan Beulich wrote:
> >>> On 02.07.2024 09:09, Cui, Lili wrote:
> >>>>> On 01.07.2024 15:42, Cui, Lili wrote:
> >>>>>> I met some issues when adding CTESTcc and CCMPcc.
> >>>>>>
> >>>>>> For example, the disassembler for "{load} ccmpbl %edx, %eax" now
> >>>>>> looks a bit
> >>>>> weird.
> >>>>>>
> >>>>>> "ccmpbl {dfv=}.s %edx,%eax"
> >>>>>
> >>>>> Well ... Something clearly needs doing about this. I'm now even
> >>>>> more convinced we want tests for these.
> >>>>>
> >>>> Indeed, I added a macro %SW to indicate operands were swapped when
> >> suffix_always is true.  Are you ok with it?
> >>>
> >>> In principle (and if then used consistently), why not. However, as
> >>> long as you use it only for ...
> >>>
> >>>> --- a/opcodes/i386-dis-evex.h
> >>>> +++ b/opcodes/i386-dis-evex.h
> >>>> @@ -940,8 +940,8 @@ static const struct dis386 evex_table[][256] = {
> >>>>      /* 38 */
> >>>>      { "%NEccmp%SCB%DF",                { Eb, Gb }, 0 },
> >>>>      { "%NEccmp%SCS%DF",                { Ev, Gv }, PREFIX_NP_OR_DATA },
> >>>> -    { "%NEccmp%SCB%DF",                { Gb, EbS }, 0 },
> >>>> -    { "%NEccmp%SCS%DF",                { Gv, EvS }, PREFIX_NP_OR_DATA },
> >>>> +    { "%NEccmp%SCB%SW%DF",     { Gb, Eb }, 0 },
> >>>> +    { "%NEccmp%SCS%SW%DF",     { Gv, Ev }, PREFIX_NP_OR_DATA },
> >>>
> >>> ... CCMPcc / CTESTcc, couldn't you make %DF fulfill this job as well?
> >>
> >> I'm sorry, this was half rubbish, I'm afraid. TEST (and hence
> >> CTESTcc) has only a single encoding, and hence would never get .s
> >> suffixes. So if there's no better way, a new %SW macro (just for
> >> CCMPcc as you have it above) it is then. An alternative might be to
> >> convert %DF to a DFV_Fixup() handler (to defer that output enough so
> >> that normal .s printing would come first), yet that may be more overhead
> than what we have right now for the {dfv=...} printing.
> >>
> >
> > I'm trying to get the opcode from ins->codep and if the opcode > 39 and
> Msuffix is ​​true, add a ".s" to it.  Do you like this version ?
> 
> I'm afraid I don't, since I consider ...
> 
> > -    { "%NEccmp%SCB%DF",                { Gb, EbS }, 0 },
> > -    { "%NEccmp%SCS%DF",                { Gv, EvS }, PREFIX_NP_OR_DATA },
> > +    { "%NEccmp%SCB%DF",                { Gb, Eb }, 0 },
> > +    { "%NEccmp%SCS%DF",                { Gv, Ev }, PREFIX_NP_OR_DATA },
> >      { Bad_Opcode },
> >      { Bad_Opcode },
> >      { Bad_Opcode },
> > --- a/opcodes/i386-dis.c
> > +++ b/opcodes/i386-dis.c
> > @@ -10587,6 +10589,11 @@ putop (instr_info *ins, const char
> *in_template, int sizeflag)
> >             }
> >           else if (l == 1 && last[0] == 'D')
> >             {
> > +             if (sizeflag & SUFFIX_ALWAYS && (ins->codep[-1] == 0X3A
> > + ||ins->codep[-1] == 0X3B))
> 
> ... this extremely fragile. It was on more than one occasion that I thought it
> might be helpful to know the major opcode while handling operands, but each
> time I found another solution. First and foremost to avoid doing what you do
> here. If we really want to allow access to the major opcode, it'll need properly
> storing (along with encoding
> space) in "ins".
> 

Yes, when I want to use major opcode for this case, I think it should be putted under ins, but I didn't find it, then I realized there might be no precedent for disassembler using major opcodes.

> As a minor remark (may save a comment on a later version), I also have
> stylistic concerns with that line (beyond it apparently being too long). Please
> can such be written as e.g.
> 
>              if ((sizeflag & SUFFIX_ALWAYS) && (ins->codep[-1] | 1) == 0x3B))
> 
> ?
Sure, it is better, this usage is common in assembler.

Based on the above discussion, I think it is better to use %sw.

Thanks,
Lili.