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

[Jan Beulich]

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.

> +  /* 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().

> +      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.

> +      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.

> +    }
> +
> +  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"?

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

Stray / leftover change?

> @@ -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.

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

Jan