[PATCH v2] Support APX CCMP and CTEST

[Jan Beulich]

On 06.06.2024 15:45, Cui, Lili wrote:
>> On 03.06.2024 12:29, Cui, Lili wrote:
>>> +static INLINE void set_oszc_flags (unsigned int oszc_shift) {
>>> +  if (i.oszc_flags & (1 << oszc_shift))
>>> +    as_bad (_("same oszc flag used twice"));
>>> +  i.oszc_flags |= 1 << oszc_shift;
>>> +}
>>
>> I also think this diagnostic would better be limited to once per parsed insn.
> 
> Do you mean to put it at the end of parse_insn ?

Well, it's not so much about where the as_bad() sits. What I care
about is that e.g. {dfv=cf,cf,cf} or {dfv=cf,cf,of,of} would have
just one such diag issued. Wherever that issuing is most suitable.

>>> +  /* Parse 'of , sf, zf, cf}'.  */
>>> +  while (*suffix_string)
>>> +    {
>>> +      if (*suffix_string == ',' || is_space_char (*suffix_string))
>>> +	suffix_string++;
>>> +      else if (*suffix_string == '}')
>>> +	{
>>> +	  suffix_string++;
>>> +	  return suffix_string - l;
>>> +	}
>>> +      else
>>> +	{
>>> +	  /* For oszc flags are updated as follows:
>>> +	     – OF = EVEX.OF
>>> +	     – SF = EVEX.SF
>>> +	     – ZF = EVEX.ZF
>>> +	     – CF = EVEX.CF
>>> +	     – PF = EVEX.CF
>>> +	     – AF = 0.  */
>>> +	  if (suffix_string[1] != 'f')
>>> +	    {
>>> +	      as_bad (_("Unrecognized oszc flags"));
>>> +	      return -1;
>>> +	    }
>>> +	  switch (suffix_string[0])
>>> +	    {
>>> +	    case 'o':
>>> +	      set_oszc_flags (OSZC_OF);
>>> +	      break;
>>> +	    case 's':
>>> +	      set_oszc_flags (OSZC_SF);
>>> +	      break;
>>> +	    case 'z':
>>> +	      set_oszc_flags (OSZC_ZF);
>>> +	      break;
>>> +	    case 'c':
>>> +	      set_oszc_flags (OSZC_CF);
>>> +	      break;
>>> +	    default:
>>> +	      as_bad (_("Unrecognized oszc flags"));
>>> +	      return -1;
>>> +	    }
>>> +	  suffix_string += 2;
>>> +	}
>>> +    }
>>> +
>>> +  as_bad (_("Unbalanced parenthesis in suffix"));
>>
>> Maybe better use `}' here. To me parenthesis means ( and ) only, but I'm aware
>> that people use that, brace, and bracket in mixed associations. Hence best to
>> be explicit about what is meant.
> 
> Changed it to '}'.  I'm not sure if it should be `}'?

Well, I'm not sure either. I merely observe that the latter form is
what's commonly used.

>> Also, question: Wouldn't it make sense to also disassemble these as simple
>> CMP/TEST (with {evex} prefix), at least by default (e.g. by slightly abusing
>> suffix-always mode to also cover the case here)?

No thoughts here?

>>> @@ -10331,6 +10348,18 @@ static const char *const fgrps[][8] = {
>>>    },
>>>  };
>>>
>>> +static const char *const oszc_flags[16] = {
>>> +  " {dfv=}", " {dfv=cf}", " {dfv=zf}", " {dfv=zf, cf}", " {dfv=sf}",
>>> +  " {dfv=sf, cf}", " {dfv=sf, zf}", " {dfv=sf, zf, cf}", " {dfv=of}",
>>> +  " {dfv=of, cf}", " {dfv=of, zf}", " {dfv=of, zf, cf}", " {dfv=of,
>>> +sf}",
>>> +  " {dfv=of, sf, cf}", " {dfv=of, sf, zf}", " {dfv=of, sf, zf, cf}"
>>> +};
>>
>> I find repeating " {dfv=" 16 times here quite odd, but I'm not going to insist
>> that you reduce this redundancy.
> 
> I try to avoid repeating 16 times with this way, but the {} is split into two parts, which looks a bit strange. What are your thoughts？
> 
>  /* Add { dfv=of, sf, zf, cf} flags.  */
> oappend (ins, "{ dfv=");
> oappend (ins, oszc_flags[oszc_value]);
> 
> static const char *const oszc_flags[16] = {
>   "}", "cf}", ...
> };

Well, if you're concerned about the (many) closing figure braces: I
don't mind if you do things as quoted above, or if you pull out
emitting of that as well.

I notice though that in the quoted code you've introduced a blank
after the emitted {. Was that on purpose?

Jan