[PATCH V2] Support APX NF

[Cui, Lili]

> -----Original Message-----
> From: Jan Beulich <jbeulich@suse.com>
> Sent: Monday, March 11, 2024 10:10 PM
> To: Cui, Lili <lili.cui@intel.com>
> Cc: hjl.tools@gmail.com; binutils@sourceware.org
> Subject: Re: [PATCH V2] Support APX NF
> 
> On 11.03.2024 14:54, Cui, Lili wrote:
> >> -----Original Message-----
> >> From: Jan Beulich <jbeulich@suse.com>
> >> Sent: Friday, March 8, 2024 5:36 PM
> >>
> >> On 04.03.2024 09:15, Cui, Lili wrote:
> >>> @@ -7218,6 +7229,11 @@ parse_insn (const char *line, char
> *mnemonic,
> >> bool prefix_only)
> >>>  		  /* {rex2} */
> >>>  		  i.rex2_encoding = true;
> >>>  		  break;
> >>> +		case Prefix_NF:
> >>> +		  /* {nf} */
> >>> +		  i.has_nf = true;
> >>> +		  i.encoding = encoding_evex;
> >>> +		  break;
> >>
> >> It's not quite as easy, I'm afraid: Have you thought of the "{vex} {nf} ..."
> >> case? (I think I previously indicated that their combination,
> >> actually in either order, needs properly rejecting.) Without having
> >> spent much thought on it, perhaps it would suffice to check here that
> >> the field is still encoding_default, and leave the value alone
> >> otherwise (in order to reject bad combinations elsewhere).
> >>
> > Oh, good point. Do you think it's ok to report bad outside of "switch"?
> >
> >                 case Prefix_NF:
> >                   /* {nf} */
> >                   i.has_nf = true;
> > -                 i.encoding = encoding_evex;
> > +                 if (i.encoding == encoding_default)
> > +                   i.encoding = encoding_evex;
> >                   break;
> >                 case Prefix_NoOptimize:
> >                   /* {nooptimize} */
> >                   i.no_optimize = true;
> >                   break;
> >                 default:
> >                   abort ();
> >                 }
> > +             if (i.has_nf && i.encoding != encoding_evex)
> > +               {
> > +                 as_bad (_("{nf} cannot be combined with {vex}/{vex3}"));
> > +                 return NULL;
> > +               }
> 
> Where the checking is done is secondary to me. My primary point is that the
> above isn't correct: There are more encoding variants than just vex, vex3, and
> evex.
> 
> That said, reporting the error right here may lead to multiple identical errors
> being reported for a single line of input. This wants avoiding, at which point
> placing the check here is ruled out.
> 

Sorry,  I didn’t get you here, do you think the current implementation is wrong? I think for parse_insn stage i.encodin only has those encoding variants, do you think we need to move further back? Maybe put them together with REX/REX2 prefix checking?

      /* Check for explicit REX prefix.  */
      if (i.prefix[REX_PREFIX] || i.rex_encoding)
        {
          as_bad (_("REX prefix invalid with `%s'"), insn_name (&i.tm));
          return;
        }

      /* Check for explicit REX2 prefix.  */
      if (i.rex2_encoding)
        {
          as_bad (_("{rex2} prefix invalid with `%s'"), insn_name (&i.tm));
          return;
        }

Thanks,
Lili.