Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(12)

Issue 175143002: Consistenly handle power-of-2 divisors in division-like operations (Closed)

Created:
6 years, 10 months ago by Sven Panne
Modified:
6 years, 9 months ago
Reviewers:
Benedikt Meurer
CC:
v8-dev
Visibility:
Public.

Description

Consistenly handle power-of-2 divisors in division-like operations Lithium currently supports 3 division-like operations on integral operands: "Normal" division (rounding towards zero), flooring division (rounding towards -Infinity) and modulus calculation (the counterpart for the "normal" division). For divisors which are a power of 2, one can efficiently use some bit fiddling to avoid the actual division for such operations. This CL cleanly splits off these operations into separate Lithium instructions, making the code much more maintainable and more consistent across platforms. There are 2 basic variations of these bit fiddling algorithms: One involving branches and a seemingly more clever one without branches. Choosing between the two is not as easy as it seems: Benchmarks (and probably real-world) programs seem to favor positive dividends, registers and shifting units are sometimes scarce resources, and branch prediction is quite good in modern processors. Therefore only the "normal" division by a power of 2 is implemented in a branch-free manner, this seems to be the best approach in practice. If this turns out to be wrong, we can easily and locally change this. R=bmeurer@chromium.org Committed: https://code.google.com/p/v8/source/detail?r=19715

Patch Set 1 #

Patch Set 2 : Avoid numerical overflow, part 1 #

Patch Set 3 : Avoid numerical overflow, part 2 #

Patch Set 4 : Back to branching code for LCodeGen::DoModByPowerOf2I on all architectures #

Patch Set 5 : Reformatted can_deopt expressions. #

Patch Set 6 : Rebased #

Patch Set 7 : Introduced LFlooringDivByPowerOf2 and LFlooringDivByConst on ia32. #

Patch Set 8 : Platform ports. Rebased. #

Patch Set 9 : Rebased, just for fun... #

Patch Set 10 : Rebased #

Patch Set 11 : Rebased. #

Patch Set 12 : Rebased #

Patch Set 13 : Rebased #

Patch Set 14 : Rebased #

Unified diffs Side-by-side diffs Delta from patch set Stats (+1444 lines, -940 lines) Patch
M src/a64/lithium-a64.h View 1 2 3 4 5 6 7 7 chunks +74 lines, -11 lines 0 comments Download
M src/a64/lithium-a64.cc View 1 2 3 4 5 6 7 8 9 10 3 chunks +101 lines, -48 lines 0 comments Download
M src/a64/lithium-codegen-a64.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 3 chunks +148 lines, -102 lines 0 comments Download
M src/arm/lithium-arm.h View 1 2 3 4 5 6 7 8 9 6 chunks +74 lines, -15 lines 0 comments Download
M src/arm/lithium-arm.cc View 1 2 3 4 5 6 7 8 9 10 3 chunks +119 lines, -76 lines 0 comments Download
M src/arm/lithium-codegen-arm.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 4 chunks +123 lines, -74 lines 0 comments Download
M src/hydrogen-instructions.h View 1 2 3 4 5 6 7 8 9 10 11 12 2 chunks +6 lines, -6 lines 0 comments Download
M src/ia32/lithium-codegen-ia32.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 1 chunk +230 lines, -224 lines 0 comments Download
M src/ia32/lithium-ia32.h View 1 2 3 4 5 6 7 8 9 6 chunks +77 lines, -11 lines 0 comments Download
M src/ia32/lithium-ia32.cc View 1 2 3 4 5 6 7 8 9 10 2 chunks +97 lines, -81 lines 0 comments Download
M src/utils.h View 1 2 1 chunk +6 lines, -0 lines 0 comments Download
M src/x64/lithium-codegen-x64.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 1 chunk +215 lines, -204 lines 0 comments Download
M src/x64/lithium-x64.h View 1 2 3 4 5 6 7 8 9 6 chunks +76 lines, -11 lines 0 comments Download
M src/x64/lithium-x64.cc View 1 2 3 4 5 6 7 8 9 10 11 12 2 chunks +98 lines, -77 lines 0 comments Download

Messages

Total messages: 3 (0 generated)
Sven Panne
The CL is actually not as big as it seems, there is a lot of ...
6 years, 9 months ago (2014-02-26 14:44:03 UTC) #1
Benedikt Meurer
LGTM, if there's test coverage for all cases changed by this CL.
6 years, 9 months ago (2014-03-03 06:39:31 UTC) #2
Sven Panne
6 years, 9 months ago (2014-03-07 10:36:40 UTC) #3
Message was sent while issue was closed.
Committed patchset #14 manually as r19715 (presubmit successful).

Powered by Google App Engine
This is Rietveld 408576698