Avoid wrong imul deopt on ia32 and x64 (fixes v8 bug 2339).

Avoid wrong imul deopt on ia32 and x64 (fixes v8 bug 2339).

BUG=v8:2339
Committed: https://code.google.com/p/v8/source/detail?r=12614

[Massi, 2012-09-21 16:19:31]

I know that this is just partially reverting r6681, but the way I read the code
it makes sense and it causes no regression.

[Vyacheslav Egorov (Google), 2012-09-21 16:51:20]

https://chromiumcodereview.appspot.com/10963032/diff/2001/src/ia32/lithium-co...
File src/ia32/lithium-codegen-ia32.cc (right):

https://chromiumcodereview.appspot.com/10963032/diff/2001/src/ia32/lithium-co...
src/ia32/lithium-codegen-ia32.cc:1237: if
(ToInteger32(LConstantOperand::cast(right)) < 0) {
If right is 0 then we need to check sign of left, which does not happen. (also
not_zero branch is redundant if right is zero). So this is not correct.

I suggest rewriting regress-1117.js in the following way:

function foo(x) { return x * 0; }

assertEquals(Infinity, 1/foo(5));
assertEquals(Infinity, 1/foo(5));
%OptimizeFunctionOnNextCall(foo);
assertEquals(-Infinity, 1/foo(-5));

And then you will see that it fails :-) It passed for you because we do OSR at
top level and then constant fold multiplication into -0.

[Massi, 2012-09-26 09:20:06]

This works also with the rewritten regression test.

[Jakob Kummerow, 2012-09-26 09:26:41]

Code changes LGTM, but I have suggestions how to improve the tests.

https://chromiumcodereview.appspot.com/10963032/diff/7001/test/mjsunit/regres...
File test/mjsunit/regress/regress-1117.js (right):

https://chromiumcodereview.appspot.com/10963032/diff/7001/test/mjsunit/regres...
test/mjsunit/regress/regress-1117.js:34: for( var i = 0; i< 1000000; i++){
While you are at it, please replace this loop with %OptimizeFunctionOnNextCall
(just have it do the same as for bar() below).

https://chromiumcodereview.appspot.com/10963032/diff/7001/test/mjsunit/regres...
File test/mjsunit/regress/regress-2339.js (right):

https://chromiumcodereview.appspot.com/10963032/diff/7001/test/mjsunit/regres...
test/mjsunit/regress/regress-2339.js:51: for (var i = 0; i < 10000; i++) {
can we use %OptimizeFunctionOnNextCall here too? --trace-opt should tell you
whether it's simple() [with inlining] or simple_two_args() that must be
optimized.

[Vyacheslav Egorov (Google), 2012-09-26 10:07:36]

dbc

https://chromiumcodereview.appspot.com/10963032/diff/7001/src/x64/lithium-cod...
File src/x64/lithium-codegen-x64.cc (right):

https://chromiumcodereview.appspot.com/10963032/diff/7001/src/x64/lithium-cod...
src/x64/lithium-codegen-x64.cc:1126: __ j(not_zero, &done, Label::kNear);
This code is redundant if right is 0. (left is always zero).