[PATCH 1/2 v2] Support APX CCMP and CTEST

[Cui, Lili]

> On 14.06.2024 13:08, Cui, Lili wrote:
> > @@ -1929,6 +1939,111 @@ static INLINE bool need_evex_encoding (const
> > insn_template *t)  #define CPU_FLAGS_PERFECT_MATCH \
> >    (CPU_FLAGS_ARCH_MATCH | CPU_FLAGS_64BIT_MATCH)
> >
> > +static INLINE bool set_oszc_flags (unsigned int oszc_shift) {
> > +  if (i.oszc_flags & oszc_shift)
> > +    {
> > +      as_bad (_("same oszc flag used twice"));
> > +      return false;
> > +    }
> > +  i.oszc_flags |= oszc_shift;
> > +  return true;
> > +}
> > +
> > +/* Handle SCC OSZC flags.  */
> > +
> > +static int
> > +check_Scc_OszcOperations (const char *l) {
> > +  const char *suffix_string = l;
> > +
> > +  while (is_space_char (*suffix_string))
> > +    suffix_string++;
> 
> As mentioned in a earlier reply, I'd prefer if these were - consistent with what we
> have elsewhere - if(), not while(). But I won't insist.
> 

I prefer to use while (), thanks.

> > +  /* If {oszc flags} is absent, just return.  */  if (*suffix_string
> > + != '{')
> > +    return 0;
> > +
> > +  /* Skip '{'.  */
> > +  suffix_string++;
> > +
> > +  /* Parse 'dfv='.  */
> > +  while (is_space_char (*suffix_string))
> > +    suffix_string++;
> > +
> > +  if (strncasecmp (suffix_string, "dfv", 3) == 0)
> > +    suffix_string += 3;
> > +  else
> > +    {
> > +      as_bad (_("Unrecognized pseudo-suffix"));
> 
> Please can diagnostics start without capital letters (unless it's some form of
> acronym)? See elsewhere, including what you have right here in set_oszc_flags().
> 

Done.

> > +      return -1;
> > +    }
> > +
> > +  while (is_space_char (*suffix_string))
> > +    suffix_string++;
> > +
> > +  if (*suffix_string == '=')
> > +    suffix_string++;
> > +  else
> > +    {
> > +      as_bad (_("Unrecognized pseudo-suffix"));
> > +      return -1;
> > +    }
> > +
> > +  /* 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;
> 
> Such an increment is pretty pointless, and hopefully the compiler will translate it
> to
> 
> 	return suffix_string + 1 - l;
> 
> anyway. Better to also write it like this right away.
>

Done.
 
> > +      if (strncasecmp (suffix_string, "of", 2) == 0)
> > +	{
> > +	  if (!set_oszc_flags (OSZC_OF))
> > +	    return -1;
> > +	}
> > +      else if (strncasecmp (suffix_string, "sf", 2) == 0)
> > +	{
> > +	  if (!set_oszc_flags (OSZC_SF))
> > +	    return -1;
> > +	}
> > +      else if (strncasecmp (suffix_string, "zf", 2) == 0)
> > +	{
> > +	  if (!set_oszc_flags (OSZC_ZF))
> > +	    return -1;
> > +	}
> > +      else if (strncasecmp (suffix_string, "cf", 2) == 0)
> > +	{
> > +	  if (!set_oszc_flags (OSZC_CF))
> > +	    return -1;
> > +	}
> > +      else
> > +	{
> > +	  as_bad (_("Unrecognized oszc flags or illegal `,' in pseudo-suffix"));
> > +	  return -1;
> > +	}
> > +
> > +      suffix_string += 2;
> > +
> > +      while (is_space_char (*suffix_string))
> > +	suffix_string++;
> > +
> > +      if (*suffix_string == '}')
> > +	return ++suffix_string - l;
> > +
> > +      if (*suffix_string != ',')
> > +	break;
> > +
> > +      suffix_string ++;
> 
> Please omit the stray / inconsistent blank.
> 

Done.

> > +    }
> > +
> > +  as_bad (_("Illegal `}' or `,' in pseudo-suffix"));
> 
> When you come here, isn't it that you _did not_ find the expected } or ,?
> IOW s/Illegal/missing"?
> 

Done.

> > @@ -7453,6 +7598,7 @@ parse_insn (const char *line, char *mnemonic, bool
> prefix_only)
> >  	    }
> >  	}
> >      }
> > +
> >    /* Any other comma loses.  */
> >    if (*l == ',')
> >      {
> 
> Stray / leftover change?
> 

Done.

> > @@ -10398,6 +10427,23 @@ putop (instr_info *ins, const char *in_template,
> int sizeflag)
> >  		*ins->obufp++ = *q;
> >  	      break;
> >  	    }
> > +	  else if (l == 1 && last[0] == 'S')
> > +	    {
> > +	      /* Add scc suffix.  */
> > +	      oappend (ins, scc_suffix[ins->vex.scc]);
> > +
> > +	      /* For SCC insns, the ND bit is required to be set to 0.  */
> > +	      if (ins->vex.nd)
> > +		oappend (ins, "(bad)");
> > +
> > +	      /* These bits have been consumed and should be cleared or restored
> > +		 to default values.  */
> > +	      ins->vex.v = 1;
> > +	      ins->vex.nf = false;
> > +	      ins->vex.mask_register_specifier = 0;
> 
> Unlike here ...
> 
> > @@ -10532,6 +10582,18 @@ putop (instr_info *ins, const char *in_template,
> int sizeflag)
> >  		  evex_printed = true;
> >  		}
> >  	    }
> > +	  else if (l == 1 && last[0] == 'D')
> > +	    {
> > +	      /* Get oszc flags value from register_specifier.  */
> > +	      int oszc_value = ~ins->vex.register_specifier & 0xf;
> > +
> > +	      /* Add {dfv=of, sf, zf, cf} flags.  */
> > +	      oappend (ins, oszc_flags[oszc_value]);
> > +
> > +	      /* These bits have been consumed and should be cleared or restored
> > +		 to default values.  */
> > +	      ins->vex.register_specifier = 0;
> 
> ... it now only "cleared" anymore here.
> 

Done.

> All adjustment requests are largely cosmetic, so with them carried out feel free
> to commit.
> 
Committed.

Thanks,
Lili.