[PATCH v4 3/9] Support APX GPR32 with extend evex prefix

[Jan Beulich]

On 25.12.2023 13:23, Cui, Lili wrote:
>> On 19.12.2023 13:12, Cui, Lili wrote:
>>>  #define Vsz256 Vsz=VSZ256
>>>  #define Vsz512 Vsz=VSZ512
>>>
>>> +// The template supports VEX format for cpuid and EVEX format for cpuid &
>> apx_f.
>>> +#define APX_F(cpuid) cpuid&(cpuid|APX_F)
>>
>> I think the comment wants to go into further detail. Please can you go back to
>> read what I said when I suggested this construct, in particular regarding the
>> stripping then done? However, with you not having found a need to fiddle
>> with cpu_flags_match(), I wonder if this construct is needed in the first place.
>> The earlier suggestion was entirely based on the assumption that stripping
>> similar to that for other combined VEX/EVEX templates would be needed here,
>> t
>>
> 
> Seeing this, I realized the problem and checked opcodes/i386-tbl.h, for the following entry, we want to set CpuAPX_F and CpuBMI to 1, but gen.c doesn't seem to support the format "cpuid&(cpuid|APX_F)",

I suppose you mean "cpuid|(cpuid&APX_F)"?

> in fact bzhi sets CpuBMI to 1 and CpuAPX_F to 0 . I'm not familiar with the relevant logic in gen.c and don't know how to debug it. when you have time, could you help take a look ?

Well, now that the patch was committed I of course can easily take a look.
But with such an outstanding issue, how could the patch have been committed
in the first place? And isn't it a requirement for making changes to
i386-gen.c that you sufficiently understand the logic there (which admittedly
has grown non-trivial)? Anyway - I will need to find time to play with what
has been committed, but that's not likely to happen in time for 2.42. Hence
it is quite possible that 2.42 will turn out to have incomplete and/or broken
APX_F support. But perhaps this was intended to be this way by H.J., when he
approved the entire series without even a single comment.

Jan