lkml.org 
[lkml]   [2015]   [Apr]   [1]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 7/9] x86/asm/entry/32: tidy up some instructions

* Denys Vlasenko <dvlasenk@redhat.com> wrote:

> After TESTs, use logically correct JZ mnemonic instead of JE
> (this doesn't change code).
>
> Tidy up CMPW insns:
>
> Modern CPUs are not good with 16-bit operations.
> The instructions with 16-bit immediates are especially bad,
> on many CPUs they cause length changing prefix stall
> in the decoders, costing ~6 cycles to recover.
>
> Replace CMPWs with CMPLs.
> Of these, for form with 8-bit sign-extended immediates
> it is a win because they are smaller now
> (no 0x66 prefix anymore);
> ones with 16-bit immediates are faster.

This patch does JE->JZ transitions, but it also does CMPW instruction
tweaking - which was buggy as Brian (miraculously!) noticed.

This isn't the first such incident, and I made this point about three
times already in the past, but it appears I've not made it loud
enough: which part of 'do not put two unrelated changes into the same
patch' did you not understand??

We _DO NOT PUT_ multiple, unrelated changes to assembly files into a
single patch! And we _especially_ don't mix them up under a
meaningless, repetitive, misleading 'tidy up instructions' title!

Full stop.

The titles of the two patches should have been something like:

x86/asm/entry/32: Convert JNE to JNZ mnemonics, to improve readability
x86/asm/entry/32: Optimize CMPW to CMPL instructions, to make use of automatic zero-extend

We were lucky that Brian was alert enough to have read through a
misleadingly titled, seemingly harmless patch and noticed the bug in
your patch, but heck you made it hard!!!

And no, it's not a problem if you create a dozen trivial looking
patches and have to wait a bit more for them to trickle into the
maintainer tree: asm patches are seldom trivial, and even if they are
trivial, both reviewability and bisectability will improve from the
process.

You are doing a nice job improving the x86/asm/entry code, but if you
cannot create suitably conservative, maximally reviewable and
maximally bisectable patches to x86/asm then I won't be able to apply
assembly patches from you!

</rant>

Thanks,

Ingo


\
 
 \ /
  Last update: 2015-04-01 11:01    [W:0.213 / U:0.760 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site