[PATCH 1/3] Support APX CCMP and CTEST

[Cui, Lili]

> -----Original Message-----
> From: Jan Beulich <jbeulich@suse.com>
> Sent: Friday, June 14, 2024 2:21 PM
> To: Cui, Lili <lili.cui@intel.com>
> Cc: hjl.tools@gmail.com; binutils@sourceware.org
> Subject: Re: [PATCH 1/3] Support APX CCMP and CTEST
> 
> On 14.06.2024 05:29, Cui, Lili wrote:
> >>>>> @@ -1929,6 +1939,114 @@ static INLINE bool need_evex_encoding
> >>>>> +  /* Parse 'of , sf, zf, cf}'.  */
> >>>>> +  while (*suffix_string)
> >>>>> +    {
> >>>>> +      if (*suffix_string == ',' || is_space_char (*suffix_string))
> >>>>> +	suffix_string++;
> >>>>
> >>>> Like for the earlier loop in the earlier version: Is it really okay
> >>>> to have multiple successive commas (with or without whitespace in
> >> between)?
> >>>
> >>> Ok, I'll add a check for it.
> >>
> >> It's not really another check that's needed. When put at the bottom
> >> of the loop body, your expectation simply is to find a brace or a
> >> comma. Anything else is an error. (That way "{dfv=,cf}" would then
> >> also be properly
> >> rejected.)
> >>
> >
> > I don't understand the logic of your approach, I think I missed some
> information. Currently I am using the following method to implement it.
> >
> > bool check_comma = true;
> >
> >   while (*suffix_string)
> >     {
> >       if (*suffix_string == ',')
> >         {
> >           /* Report an error for illegal commas.  */
> >           if (check_comma == true)
> >             {
> >               as_bad (_("Illegal comma found in pseudo-suffix"));
> >               return -1;
> >             }
> >           check_comma = true;
> >           suffix_string++;
> >         }
> >       else if (is_space_char (*suffix_string))
> >         suffix_string++;
> >       else if (*suffix_string == '}')
> >         {
> >           suffix_string++;
> >           return suffix_string - l;
> >         }
> >       else
> >         {
> >           check_comma = false;
> >           ...
> >           suffix_string += 2;
> >         }
> >     }
> 
> (just an outline)
> 
>   while (*suffix_string)
>     {
>       if (is_space_char (*suffix_string))
>         suffix_string++;
>       if (TOLOWER (suffix_string[1]) != 'f')
>         break;
>       ...
>       suffix_string += 2;
> 
>       if (is_space_char (*suffix_string))
>         suffix_string++;
>       if (*suffix_string == '}')
>         return suffix_string + 1 - l;
>       if (*suffix_string != ',')
>         break;
>       suffix_string++;
>     }
> 
>   as_bad (...);
>   return NULL;
> 
> The only special case not covered (perhaps needing an extra check ahead of
> the loop) is "{dfv=}", which I have to admit I'm not entirely certain needs
> supporting at all (for being the same as omitting the construct altogether). Yet
> I'm certainly okay to permit it, allowing people to write what some may call
> more explicit code.
> 

Added `{` check for "{dfv=}" before oszc flags check. Also removed previous loop (parsing 'dfv=' ) to keep the same style in this function.

/* Parse 'of, sf, zf, cf}'.  */
  while (*suffix_string)
    {
      while (is_space_char (*suffix_string))
        suffix_string++;

      /* Return for '{dfv=}'.  */
      if (*suffix_string == '}')
        return ++suffix_string - l;

      if (strncasecmp (suffix_string, "of", 2) == 0)
        {
           ...
        }

Thanks,
Lili.