Re: [Xen-devel] [PATCH 01/17] x86emul: support remaining AVX insns

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 13.09.17 at 17:02, <dunlapg@xxxxxxxxx> wrote:
> On Wed, Jun 21, 2017 at 12:59 PM, Jan Beulich <JBeulich@xxxxxxxx> wrote:
>> I.e. those not being equivalents of SSEn ones.
>>
>> There's one necessary change to generic code: Faulting behavior of
>> VMASKMOVP{S,D} requires us to do partial reads/writes.
>>
>> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
> 
> Having dug a bit through the manuals about AVX, then come back to this
> patch again, I feel the same way that I did when I first read it:
> This discription is woefully inadequate.  It doesn't look so much like
> a changelog as a note to yourself.  If someone else sent a patch
> description like this for a patch this complicated and expected you to
> review it you'd certainly complain, and there's a good chance you
> wouldn't review it.
> 
> It's not your job to teach me how this code is laid out and how it
> works, but there at least needs to be enough description of what the
> patch is actually trying to do that I have some hope of reconstructing
> its intent.

Hmm, for someone not familiar with prior changes in this direction
(SSE etc) I can see how the brevity might be confusing / unhelpful.
However, the title of the patch says all that is to be said about
"its intent". The description really just points out the one thing
that is not in line with code that's already there. And I'm sure you
don't mean me to enumerate the individual instructions support is
being added for. Yet beyond that and beyond the partial write
handling there's really nothing the patch does. So I'm sort of lost
as to what additional information I could provide.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel