Handle non-constant divisor in MathFloorOfDiv, on ia32/x64

Handle non-constant divisor in MathFloorOfDiv, on ia32/x64

Zheng Liu
zheng.z.liu@intel.com

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

[Zheng Liu, 2012-12-21 09:13:37]

PTAL

[Yang, 2012-12-27 14:41:53]

Got some comments.

https://codereview.chromium.org/11624022/diff/6001/src/ia32/lithium-codegen-i...
File src/ia32/lithium-codegen-ia32.cc (right):

https://codereview.chromium.org/11624022/diff/6001/src/ia32/lithium-codegen-i...
src/ia32/lithium-codegen-ia32.cc:1296: Label done;
Tests, especially targetting this code path, would be really great. There are
many cases to test: positive/negative left and right, 0 in quotient and/or
remainder, etc.

https://codereview.chromium.org/11624022/diff/6001/src/ia32/lithium-ia32.cc
File src/ia32/lithium-ia32.cc (right):

https://codereview.chromium.org/11624022/diff/6001/src/ia32/lithium-ia32.cc#n...
src/ia32/lithium-ia32.cc:1309: if (divisor->representation().IsInteger32())
For if-else, we always insert brackets.

https://codereview.chromium.org/11624022/diff/6001/src/ia32/lithium-ia32.cc#n...
src/ia32/lithium-ia32.cc:1310: return divisor;
Please conform to the usual code style. Specifically, use no brackets if the if
is a one-liner, or brackets if you have line breaks.

https://codereview.chromium.org/11624022/diff/6001/src/x64/lithium-codegen-x6...
File src/x64/lithium-codegen-x64.cc (right):

https://codereview.chromium.org/11624022/diff/6001/src/x64/lithium-codegen-x6...
src/x64/lithium-codegen-x64.cc:1233: __ addl(rax, rdx);
Ditto, tests would be great.

https://codereview.chromium.org/11624022/diff/6001/src/x64/lithium-x64.cc
File src/x64/lithium-x64.cc (right):

https://codereview.chromium.org/11624022/diff/6001/src/x64/lithium-x64.cc#new...
src/x64/lithium-x64.cc:1236: HChange::cast(divisor)->from().IsInteger32())
Please indent the second line of the if-condition like we do in line 1226, and
add brackets for the if-body.

[Zheng Liu, 2012-12-28 00:18:26]

Fixed format and updated test.
math-floor-of-div-nosudiv is also updated as a copy of math-floor-of-div

[Yang, 2012-12-28 15:51:55]

On 2012/12/28 00:18:26, Zheng Liu wrote:
> Fixed format and updated test.
> math-floor-of-div-nosudiv is also updated as a copy of math-floor-of-div

LGTM. Landing.