Re: [Xen-devel] [PATCH 1/2] x86emul: fix vector-length check for AVX512F scalar fused-multiply-add insns

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 10.12.18 at 15:26, <andrew.cooper3@xxxxxxxxxx> wrote:
> On 10/12/2018 14:11, Jan Beulich wrote:
>>>>> On 10.12.18 at 14:50, <andrew.cooper3@xxxxxxxxxx> wrote:
>>> On 07/12/2018 11:17, Jan Beulich wrote:
>>>> The check needs to happen whenever EVEX.b is clear, not just in the
>>>> memory operand case.
>>> EVEX.b is a different field to EVEX.br
>>>
>>> I'm afraid that this goes back to my original concern with the series. 
>>> Having the fields named differently between our code and the manual is
>>> now causing problems for everyone.
>> I'm trying to be very strict with distinguishing things in text:
>> EVEX.B is what in code we call evex.b; EVEX.b is what in code
>> we call evex.br. There's no such thing as EVEX.br. I strictly
>> mean to make the textual (outside of code) distinction via
>> case: EVEX is to be used with SDM names, evex with our code's
>> ones.
>>
>> I thought we had meanwhile settled this dispute, realizing
>> that there simply is no really good way of naming things in
>> code without some (undesirable) deviation from the SDM.
> 
> The fact that we don't have a better option does not mean that the
> confusion has stopped.  The fact that there are one and a half people in
> the world who can read this code means that it will keep being a problem
> until we find some better option.
> 
> As for this patch, this is the first time I've (knowingly) encountered
> your scheme for disambiguation.  I've not (knowingly) seen it written
> down or pointed out anywhere.
> 
> Basically - the commit message and the patch content contradict each
> other, and it is not reasonable to expect readers to know that EVEX in
> the commit message bears no relationship to evex in the code.

How about

"The check needs to happen whenever EVEX.b (SDM nomenclature) is clear,
 not just in the memory operand case."

as description then? I've just checked other patches: There talk is
of EVEX.W or EVEX.L'L, in which case it is more obvious that case
doesn't matter (EVEX.W) or the SDM naming is used (EVEX.L'L), so
I'd prefer not to add similar clarifications there.

Jan



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel