[PATCH] Support APX CCMP and CTEST

[Jan Beulich]

On 29.05.2024 10:44, Cui, Lili wrote:
>> On 29.05.2024 08:37, Cui, Lili wrote:
>>>> On 23.05.2024 08:12, Cui, Lili wrote:
>>>>> +	  if (op_string[1] != 'f')
>>>>> +	    {
>>>>> +	      as_bad (_("Unrecognized oszc flags"));
>>>>> +	      ignore_rest_of_line ();
>>>>> +	      return NULL;
>>>>> +	    }
>>>>> +	  switch (op_string[0])
>>>>> +	    {
>>>>> +	    case 'o':
>>>>> +	      i.oszc_flags |= (1 << OF);
>>>>> +	      break;
>>>>> +	    case 's':
>>>>> +	      i.oszc_flags |= (1 << SF);
>>>>> +	      break;
>>>>> +	    case 'z':
>>>>> +	      i.oszc_flags |= (1 << ZF);
>>>>> +	      break;
>>>>> +	    case 'c':
>>>>> +	    case 'p':
>>>>
>>>> I don't think "pf" should be recognized here. As terminology says
>>>> here and in the spec, it's OSZC (no P in there).
>>>
>>>>> +	    case 'a':
>>>>> +	      break;
>>>>
>>>> "af" pretty certainly may not be recognized here, as that would imply
>>>> EFLAGS.AF becoming set, not cleared.
>>>>
>>>
>>> I know what you mean, SCC cannot test PF.
>>>
>>> • If SCC = 0b1010, then SCC evaluates to true regardless of the status flags
>> value.
>>> • If SCC = 0b1011, then SCC evaluates to false regardless of the status flags
>> value.
>>> Consequently, the SCC cannot test the parity flag PF.
>>
>> All of the code here is solely about OSZC; I don't see why you bring SCC into
>> the picture right here.
>>
>>> But they are listed in another place, and we should assign PF to EVEX.CF and
>> no update for AF.
>>>
>>> • If SCC evaluates to false on the status flags, then the CMP or TEST
>>> is not executed and instead the status flags are updated as follows:
>>> – OF = EVEX.OF
>>> – SF = EVEX.SF
>>> – ZF = EVEX.ZF
>>> – CF = EVEX.CF
>>> – PF = EVEX.CF
>>> – AF = 0
>>
>> Yes. But still even in what you write above it's EVEX.CF. There's no EVEX.PF, and
>> hence there also shouldn't be {dfv=pf}. (To be honest I would have found it
>> clearer if PF, like AF, was simply cleared. But there are likely reasons for it not
>> being that way. Sadly such reasoning is never made publicly available ...)
>>
> 
> You are right, I should remove PF and AF here. I think it wants to give users a chance to set PF.
> 
>>>>
>>>>> +	      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.

Jan