[PATCH] Support APX CCMP and CTEST

[Jan Beulich]

On 30.05.2024 04:44, Cui, Lili wrote:
>> On 29.05.2024 13:05, Cui, Lili wrote:
>>>>>>>>> +	      i.oszc_flags |= (1 << CF);
>>>>>>>>> +	      break;
>>>>>>>>
>>>>>>>> Shouldn't you further reject redundant settings, as in
>>>>>>>>
>>>>>>>> 	ccmpe {dfv=cf,cf} ...
>>>>>>>>
>>>>>>>> ?
>>>>>>>
>>>>>>> How about keeping this compatibility? Like any other pseudo prefix.
>>>>>>
>>>>>> Which "compatibility"? And why the reference to pseudo prefixes
>>>>>> when here we're dealing with something entirely new, a pseudo suffix?
>>>>>> Within a single {dfv=...} each flag should be mentioned at most once.
>>>>>> Anything else is a potential indication of a mistake the programmer made.
>>>>>> Separately from this we may consider whether to permit more than
>>>>>> one {dfv=...} for a single insn, with the latter than fully
>>>>>> replacing the former's effects. Personally I'd recommend against
>>>>>> that unless a clear use case could be provided, but I wouldn't
>>>>>> object to such being done
>>>> right away.
>>>>>>
>>>>>
>>>>> It's a suffix, but I think we can tolerate multiple repetitions of a
>>>>> flag by just
>>>> overwriting the previous one with the next one. Like the
>>>> pseudo-prefix {vex} {vex} . If you insist on giving {dfv=cf,cf}  an
>>>> error, I'd just put a check before the assignment.
>>>>
>>>> Well, if you think it may be useful, make it just a warning? Part of
>>>> my desire to see this diagnosed is related to your use of "overwriting" in the
>> reply:
>>>> There's no real overwriting; things can only be cumulative here, as
>>>> there's no way to specify the clearing of one of the flags. Any flags
>>>> to be cleared are indicated as such by simply not mentioning them at all.
>>>>
>>>
>>> Yes, "overright" is not the right word. How about this?
>>>
>>> +static INLINE void set_oszc_flags (unsigned int oszc_shift) {
>>> +  if (i.oszc_flags && (1 << oszc_shift))
>>
>> With && replaced by & here ...
>>
> 
> Sorry, I pasted a wrong version here.
> 
>>> +    as_bad (_("same oszc flag used twice"));
>>> +  i.oszc_flags |= 1 << oszc_shift;
>>> +}
>>> +
>>>  /* Handle SCC OSZC flags.  */
>>>
>>>  static int
>>> @@ -1999,16 +2006,16 @@ check_Scc_OszcOperations (const char *l)
>>>            switch (suffix_string[0])
>>>              {
>>>              case 'o':
>>> -              i.oszc_flags |= (1 << OF);
>>> +             set_oszc_flags (OF);
>>>                break;
>>>              case 's':
>>> -              i.oszc_flags |= (1 << SF);
>>> +             set_oszc_flags (SF);
>>>                break;
>>>              case 'z':
>>> -              i.oszc_flags |= (1 << ZF);
>>> +             set_oszc_flags (ZF);
>>>                break;
>>>              case 'c':
>>> -              i.oszc_flags |= (1 << CF);
>>> +             set_oszc_flags (CF);
>>>                break;
>>>              default:
>>
>> ... this looks okay to me (still pending the rename of the constant names,
>> though).
> 
> I think you mean const char *l (suffix_string),

No, I mean the CF, ZF, etc constants.

Jan

> I moved this function to parse_insn, and it needs to return a value for length (suffix_string - l).  More details will be provided in the next version. 
> 
> Thanks,
> Lili.
>