[PATCH] Support APX CCMP and CTEST
Jan Beulich
jbeulich@suse.com
Wed May 29 12:25:03 GMT 2024
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 ...
> + 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).
Jan
More information about the Binutils
mailing list