[PATCH V2] Support APX CFCMOV

[Cui, Lili]

> On 01.07.2024 08:37, Cui, Lili wrote:
> >> On 25.06.2024 11:56, Cui, Lili wrote:
> >>> @@ -14039,3 +14047,26 @@ JMPABS_Fixup (instr_info *ins, int
> >>> bytemode,
> >> int sizeflag)
> >>>      return OP_IMREG (ins, bytemode, sizeflag);
> >>>    return OP_OFF64 (ins, bytemode, sizeflag);  }
> >>> +
> >>> +static bool
> >>> +CFCMOV_Fixup_op (instr_info *ins, int opnd, int sizeflag) {
> >>> +  /* EVEX.NF is used as a direction bit in the 2-operand case to reverse
> the
> >>> +     source and destination operands.  */
> >>> +  if (!ins->vex.nd && ins->vex.nf)
> >>> +    {
> >>> +      if (opnd == 0)
> >>> +	return OP_E (ins, v_swap_mode, sizeflag);
> >>
> >> There's still no testing of this use of v_swap_mode afaics.
> >>
> >
> > I thought adding -Msuffix in x86-64-apx-cfcmov-intel.d and doing the
> following tests were the tests you wanted, now it seems that I'm missing
> something. Could you add more information? Thanks.
> >
> > [       ]*[a-f0-9]+:[   ]*62 f4 7d 08 42 c2[    ]+cfcmovb ax,dx
> > [       ]*[a-f0-9]+:[   ]*62 f4 7d 0c 42 d0[    ]+cfcmovb.s ax,dx
> > [       ]*[a-f0-9]+:[   ]*62 f4 7d 08 42 c2[    ]+cfcmovb ax,dx
> > [       ]*[a-f0-9]+:[   ]*62 f4 7d 0c 42 d0[    ]+cfcmovb.s ax,dx
> 
> Hmm, I overlooked that, first and foremost because I certainly did not expect
> this to be in an Intel syntax test. The above is not really Intel syntax, and -
> Msuffix also doesn't make a lot of sense together with -Mintel. (Read:
> Eventually we may want to actually reject that combination, or re-purpose it
> to make the "<xyz> PTR" printing optional when operand size is unambiguous.
> Nevertheless I can also see the desire to have a way to express the non-default
> encodings in Intel syntax output.)
> 

Would you mind moving these four tests into " x86: Add {load}/{store} tests for apx instructions"?  then we can remove -Msuffix in Intel syntax tests.

> >>> +      /* These bits have been consumed and should be cleared.  */
> >>> +      ins->vex.nf = false;
> >>> +      ins->vex.mask_register_specifier = 0;
> >>
> >> Hmm, I thought I had asked for this and ...
> >>
> >>> +      return OP_G (ins, v_mode, sizeflag);
> >>> +    }
> >>> +
> >>> +  if (opnd == 0)
> >>> +    return OP_G (ins, v_mode, sizeflag);
> >>> +  /* These bits have been consumed and should be cleared.  */
> >>> + ins->vex.nf = false;  ins->vex.mask_register_specifier = 0;
> >>
> >> ... this state update to be folded; I may be misremembering though.
> >> Any future updating shouldn't require touching two entirely identical
> >> places in the same
> >> (small) function. Plus when taking the first OP_E() path, you fail to
> >> clear -
> >>> vex.nf right now anyway (i.e. another reason to do it once uniformly).
> >>
> >
> > I think you mean to clear vex.nf and vex.mask_register_specifier in only one
> place.
> 
> Indeed.
> 
> > I checked the logic, I think we can't fold them, we want to clear
> > vex.nf before returning the last operand, the last operand has two
> > cases,
> >
> > Case 1: (!ins->vex.nd && ins->vex.nf) is true, and vex.nf needs to be cleared
> after judgment.
> > Case 2: ins->vex.nf is true, and vex.nf needs to be cleared.( I added
> > a condition to clear it exactly)
> 
> I don't see what good that condition does. It only make the code bigger.
> 
> >    if (opnd == 0)
> >      return OP_G (ins, v_mode, sizeflag);
> > -  /* These bits have been consumed and should be cleared.  */
> > -  ins->vex.nf = false;
> > -  ins->vex.mask_register_specifier = 0;
> > +  if (ins->vex.nf)
> > +    {
> > +      /* These bits have been consumed and should be cleared.  */
> > +      ins->vex.nf = false;
> > +      ins->vex.mask_register_specifier = 0;
> > +    }
> >    return OP_E (ins, v_mode, sizeflag);  }
> 
> static bool
> CFCMOV_Fixup_op (instr_info *ins, int opnd, int sizeflag) {
>   /* EVEX.NF is used as a direction bit in the 2-operand case to reverse the
>      source and destination operands.  */
>   bool dstmem = !ins->vex.nd && ins->vex.nf;
> 
>   if (opnd == 0)
>     {
>       if (dstmem)
> 	return OP_E (ins, v_swap_mode, sizeflag);
>       return OP_G (ins, v_mode, sizeflag);
>     }
> 
>   /* These bits have been consumed and should be cleared.  */
>   ins->vex.nf = false;
>   ins->vex.mask_register_specifier = 0;
> 

I tried this version as well, but I didn't record vex.nf before clearing it, so the folding failed in the end. 

Thanks,
Lili.

>   if (dstmem)
>     return OP_G (ins, v_mode, sizeflag);
>   return OP_E (ins, v_mode, sizeflag);
> }
> 
> Jan