Chromium Code Reviews
Help | Chromium Project | Sign in
(441)

Issue 9638018: [v8-dev] Optimise Math.floor(x/y) to use integer division for specific divisor.... (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
2 years, 1 month ago by Rodolph Perfetta
Modified:
1 year, 12 months ago
CC:
v8-dev_googlegroups.com
Visibility:
Public.

Description

Optimise Math.floor(x/y) to use integer division for specific divisor.

BUG=none
TEST=mjsunit/math-floor-of-div.js

Landing for Rodolph Perfetta <rodolph.perfetta@gmail.com>;.
Committed: https://code.google.com/p/v8/source/detail?r=11420

Patch Set 1 #

Total comments: 20

Patch Set 2 : '' #

Total comments: 8

Patch Set 3 : '' #

Patch Set 4 : '' #

Unified diffs Side-by-side diffs Delta from patch set Stats (+628 lines, -24 lines) Lint Patch
M src/arm/lithium-arm.h View 1 2 3 3 chunks +20 lines, -0 lines 0 comments 0 errors Download
M src/arm/lithium-arm.cc View 1 2 3 1 chunk +69 lines, -0 lines 0 comments 0 errors Download
M src/arm/lithium-codegen-arm.h View 1 2 3 1 chunk +11 lines, -0 lines 0 comments 0 errors Download
M src/arm/lithium-codegen-arm.cc View 1 2 3 2 chunks +122 lines, -0 lines 0 comments 0 errors Download
M src/arm/macro-assembler-arm.h View 1 2 3 1 chunk +8 lines, -1 line 0 comments 0 errors Download
M src/arm/macro-assembler-arm.cc View 1 2 3 1 chunk +21 lines, -8 lines 0 comments 0 errors Download
M src/compiler-intrinsics.h View 1 2 3 3 chunks +24 lines, -0 lines 0 comments 0 errors Download
M src/frames.cc View 1 2 3 1 chunk +1 line, -6 lines 0 comments 0 errors Download
M src/hydrogen-instructions.h View 1 2 3 3 chunks +21 lines, -9 lines 0 comments 0 errors Download
M src/hydrogen-instructions.cc View 1 2 3 1 chunk +56 lines, -0 lines 0 comments 0 errors Download
M src/ia32/lithium-ia32.cc View 1 2 3 1 chunk +6 lines, -0 lines 0 comments 0 errors Download
M src/mips/lithium-mips.cc View 1 2 3 1 chunk +6 lines, -0 lines 0 comments 0 errors Download
M src/utils.h View 1 2 3 1 chunk +26 lines, -0 lines 0 comments 0 errors Download
M src/utils.cc View 1 2 3 1 chunk +15 lines, -0 lines 0 comments 0 errors Download
M src/x64/lithium-x64.cc View 1 2 3 1 chunk +6 lines, -0 lines 0 comments 0 errors Download
A test/mjsunit/math-floor-of-div.js View 1 2 1 chunk +216 lines, -0 lines 0 comments 0 errors Download
Trybot results:
Commit:

Messages

Total messages: 18
Rodolph Perfetta
This supercedes our "optimise bitwise operations" patch.
2 years, 1 month ago #1
danno
Adding Florian, who should review this
2 years, 1 month ago #2
fschneider
On 2012/03/09 11:39:09, danno wrote: > Adding Florian, who should review this I did not ...
2 years, 1 month ago #3
fschneider
Sorry for the long delay. Here are my comments: The approach looks good. http://codereview.chromium.org/9638018/diff/1/src/arm/lithium-codegen-arm.cc File ...
2 years ago #4
Alexandre
Thanks for the review. I addressed your comments and created an issue (2038) for the ...
2 years ago #5
Rodolph Perfetta
new patch with comments addressed
2 years ago #6
Alexandre
Hello, Any news on this? Thanks, Alexandre
2 years ago #7
fschneider
LGTM. Two small comments. I can fix them before landing if it makes sense. What ...
2 years ago #8
Alexandre
I agree with your comments. It is great if you can fix them and land. ...
2 years ago #9
alexeif
https://chromiumcodereview.appspot.com/9638018/diff/13001/src/utils.h File src/utils.h (right): https://chromiumcodereview.appspot.com/9638018/diff/13001/src/utils.h#newcode112 src/utils.h:112: const DivMagicNumbers DivMagicNumberFor625 = {0x68db8bad, 8}; How about adding ...
2 years ago #10
Alexandre
https://chromiumcodereview.appspot.com/9638018/diff/13001/src/utils.h File src/utils.h (right): https://chromiumcodereview.appspot.com/9638018/diff/13001/src/utils.h#newcode112 src/utils.h:112: const DivMagicNumbers DivMagicNumberFor625 = {0x68db8bad, 8}; We can generate ...
2 years ago #11
alexeif
https://chromiumcodereview.appspot.com/9638018/diff/13001/src/utils.h File src/utils.h (right): https://chromiumcodereview.appspot.com/9638018/diff/13001/src/utils.h#newcode112 src/utils.h:112: const DivMagicNumbers DivMagicNumberFor625 = {0x68db8bad, 8}; On 2012/04/17 17:12:22, ...
2 years ago #12
Alexandre
https://chromiumcodereview.appspot.com/9638018/diff/13001/src/utils.h File src/utils.h (right): https://chromiumcodereview.appspot.com/9638018/diff/13001/src/utils.h#newcode112 src/utils.h:112: const DivMagicNumbers DivMagicNumberFor625 = {0x68db8bad, 8}; Fair question! I ...
2 years ago #13
Zheng Liu
Hi. I proposed a x86/x64 implementation in issue 2038: http://code.google.com/p/v8/issues/detail?id=2038 which depends on this patch. ...
2 years ago #14
Rodolph Perfetta
comments addressed.
2 years ago #15
fschneider
LGTM. We'll land it.
2 years ago #16
Rodolph Perfetta
Comments addressed.
1 year, 12 months ago #17
fschneider
1 year, 12 months ago #18
Thanks. Still LGTM.

I'll re-land it
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld 1280:2d3e6564b7b6