Require CMOV support for the ia32 port.

Require CMOV support for the ia32 port.

R=svenpanne@chromium.org

Committed: https://code.google.com/p/v8/source/detail?r=21279

[titzer, 2014-05-12 12:40:32]

https://codereview.chromium.org/275253004/diff/1/src/ia32/assembler-ia32.h
File src/ia32/assembler-ia32.h (left):

https://codereview.chromium.org/275253004/diff/1/src/ia32/assembler-ia32.h#ol...
src/ia32/assembler-ia32.h:726: void cmov(Condition cc, Register dst, Register
src) {
Please don't remove these from the MacroAssembler!

[Yang, 2014-05-12 12:47:32]

On 2014/05/12 12:40:32, titzer wrote:
> https://codereview.chromium.org/275253004/diff/1/src/ia32/assembler-ia32.h
> File src/ia32/assembler-ia32.h (left):
> 
>
https://codereview.chromium.org/275253004/diff/1/src/ia32/assembler-ia32.h#ol...
> src/ia32/assembler-ia32.h:726: void cmov(Condition cc, Register dst, Register
> src) {
> Please don't remove these from the MacroAssembler!

Alright. Restored them in the MacroAssembler. But since we are removing the CMOV
CPUFeature, I'll require CMOV to be supported even though we don't use it. That
should be fine though since we already require SSE2 support.

[Sven Panne, 2014-05-12 12:55:45]

https://codereview.chromium.org/275253004/diff/1/src/ia32/assembler-ia32.h
File src/ia32/assembler-ia32.h (left):

https://codereview.chromium.org/275253004/diff/1/src/ia32/assembler-ia32.h#ol...
src/ia32/assembler-ia32.h:726: void cmov(Condition cc, Register dst, Register
src) {
On 2014/05/12 12:40:32, titzer wrote:
> Please don't remove these from the MacroAssembler!

Whatever uses that should consider *not* using it. ;-) This introduces data
dependencies, and normally modern processors are better off using caveman-style
conditional branches + normal loads.

[Yang, 2014-05-12 13:00:47]

On 2014/05/12 12:55:45, Sven Panne wrote:
> https://codereview.chromium.org/275253004/diff/1/src/ia32/assembler-ia32.h
> File src/ia32/assembler-ia32.h (left):
> 
>
https://codereview.chromium.org/275253004/diff/1/src/ia32/assembler-ia32.h#ol...
> src/ia32/assembler-ia32.h:726: void cmov(Condition cc, Register dst, Register
> src) {
> On 2014/05/12 12:40:32, titzer wrote:
> > Please don't remove these from the MacroAssembler!
> 
> Whatever uses that should consider *not* using it. ;-) This introduces data
> dependencies, and normally modern processors are better off using
caveman-style
> conditional branches + normal loads.

Of course I could change them into true macros: cmov would internally use jump
and mov.

[Yang, 2014-05-12 13:41:36]

On 2014/05/12 13:00:47, Yang wrote:
> On 2014/05/12 12:55:45, Sven Panne wrote:
> > https://codereview.chromium.org/275253004/diff/1/src/ia32/assembler-ia32.h
> > File src/ia32/assembler-ia32.h (left):
> > 
> >
>
https://codereview.chromium.org/275253004/diff/1/src/ia32/assembler-ia32.h#ol...
> > src/ia32/assembler-ia32.h:726: void cmov(Condition cc, Register dst,
Register
> > src) {
> > On 2014/05/12 12:40:32, titzer wrote:
> > > Please don't remove these from the MacroAssembler!
> > 
> > Whatever uses that should consider *not* using it. ;-) This introduces data
> > dependencies, and normally modern processors are better off using
> caveman-style
> > conditional branches + normal loads.
> 
> Of course I could change them into true macros: cmov would internally use jump
> and mov.

So... what do we want now? Use cmov where possible, or never use cmov? In either
case, we would require cmov-support anyways. By now I have a CL for either
option under review...

[titzer, 2014-05-12 13:46:31]

On 2014/05/12 13:41:36, Yang wrote:
> On 2014/05/12 13:00:47, Yang wrote:
> > On 2014/05/12 12:55:45, Sven Panne wrote:
> > > https://codereview.chromium.org/275253004/diff/1/src/ia32/assembler-ia32.h
> > > File src/ia32/assembler-ia32.h (left):
> > > 
> > >
> >
>
https://codereview.chromium.org/275253004/diff/1/src/ia32/assembler-ia32.h#ol...
> > > src/ia32/assembler-ia32.h:726: void cmov(Condition cc, Register dst,
> Register
> > > src) {
> > > On 2014/05/12 12:40:32, titzer wrote:
> > > > Please don't remove these from the MacroAssembler!
> > > 
> > > Whatever uses that should consider *not* using it. ;-) This introduces
data
> > > dependencies, and normally modern processors are better off using
> > caveman-style
> > > conditional branches + normal loads.
> > 
> > Of course I could change them into true macros: cmov would internally use
jump
> > and mov.
> 
> So... what do we want now? Use cmov where possible, or never use cmov? In
either
> case, we would require cmov-support anyways. By now I have a CL for either
> option under review...

I view the assembler is a grab bag of all the things the machine can do.

I am neutral on the usage of CMOV; I just don't want to see them removed from
the Assembler, because there are sometimes good reasons to use them. If you
remove current uses, then maybe we need them later again. When that happens
someone puts them back in, then sometime later someone changes that code,
notices the methods are dead again, removes them again, on and on...

[Sven Panne, 2014-05-13 06:25:20]

On 2014/05/12 13:46:31, titzer wrote:

> [...] I am neutral on the usage of CMOV; I just don't want to see them removed
from
> the Assembler, because there are sometimes good reasons to use them. If you
> remove current uses, then maybe we need them later again. When that happens
> someone puts them back in, then sometime later someone changes that code,
> notices the methods are dead again, removes them again, on and on...

I don't think that we should remove CMOV from the assembler, the question really
is: Should we use it? Perhaps we need to have a closer look at the places where
we actually use it, and choose one version or the other, but we should not
depend on the availability of CMOV anymore. Perhaps the "Intel 64 and IA-32
Architectures Optimization Reference Manual" gives some guidelines:

---------------------------------------------
Assembly/Compiler Coding Rule 2. (M impact, ML generality) Use the SETCC and
CMOV
instructions to eliminate unpredictable conditional branches where possible. Do
not do this for
predictable branches. Do not use these instructions to eliminate all
unpredictable conditional branches
(because using these instructions will incur execution overhead due to the
requirement for executing
both paths of a conditional branch). In addition, converting a conditional
branch to SETCC or CMOV
trades off control flow dependence for data dependence and restricts the
capability of the out-of-order
engine. When tuning, note that all Intel 64 and IA-32 processors usually have
very high branch
prediction rates. Consistently mispredicted branches are generally rare. Use
these instructions only if
the increase in computation time is less than the expected cost of a
mispredicted branch.

[Some examples follow]
---------------------------------------------

And some discussions:

http://yarchive.net/comp/linux/cmov.html
https://mail.mozilla.org/pipermail/tamarin-devel/2008-April/000454.html
http://msinilo.pl/blog/?p=1115

So the general theme seems to be: If the branch is very predictable, use a
conditional branch + load, otherwise use SETCC/CMOV. Things vary a bit depending
how much you can squeeze in before you need the data dependencies. I think that
the "compacter code" argument is bogus, we have far bigger gains almost
everywhere if we really care about code size. SETCC/CMOV is an out-of-order
execution killer, this should be our biggest concern.

[Yang, 2014-05-13 07:52:55]

On 2014/05/13 06:25:20, Sven Panne wrote:
> On 2014/05/12 13:46:31, titzer wrote:
> 
> > [...] I am neutral on the usage of CMOV; I just don't want to see them
removed
> from
> > the Assembler, because there are sometimes good reasons to use them. If you
> > remove current uses, then maybe we need them later again. When that happens
> > someone puts them back in, then sometime later someone changes that code,
> > notices the methods are dead again, removes them again, on and on...
> 
> I don't think that we should remove CMOV from the assembler, the question
really
> is: Should we use it? Perhaps we need to have a closer look at the places
where
> we actually use it, and choose one version or the other, but we should not
> depend on the availability of CMOV anymore. Perhaps the "Intel 64 and IA-32
> Architectures Optimization Reference Manual" gives some guidelines:
> 
> ---------------------------------------------
> Assembly/Compiler Coding Rule 2. (M impact, ML generality) Use the SETCC and
> CMOV
> instructions to eliminate unpredictable conditional branches where possible.
Do
> not do this for
> predictable branches. Do not use these instructions to eliminate all
> unpredictable conditional branches
> (because using these instructions will incur execution overhead due to the
> requirement for executing
> both paths of a conditional branch). In addition, converting a conditional
> branch to SETCC or CMOV
> trades off control flow dependence for data dependence and restricts the
> capability of the out-of-order
> engine. When tuning, note that all Intel 64 and IA-32 processors usually have
> very high branch
> prediction rates. Consistently mispredicted branches are generally rare. Use
> these instructions only if
> the increase in computation time is less than the expected cost of a
> mispredicted branch.
> 
> [Some examples follow]
> ---------------------------------------------
> 
> And some discussions:
> 
> http://yarchive.net/comp/linux/cmov.html
> https://mail.mozilla.org/pipermail/tamarin-devel/2008-April/000454.html
> http://msinilo.pl/blog/?p=1115
> 
> So the general theme seems to be: If the branch is very predictable, use a
> conditional branch + load, otherwise use SETCC/CMOV. Things vary a bit
depending
> how much you can squeeze in before you need the data dependencies. I think
that
> the "compacter code" argument is bogus, we have far bigger gains almost
> everywhere if we really care about code size. SETCC/CMOV is an out-of-order
> execution killer, this should be our biggest concern.

It seems that all the uses of cmov seems justified. So I restored their uses.
Performance tests results are neutral either way.

[Sven Panne, 2014-05-13 08:00:28]

lgtm

[Yang, 2014-05-13 08:16:43]

Committed patchset #3 manually as r21279 (presubmit successful).