Chromium Code Reviews

Issue 166793002: Fixed and improved code for integral division. Fixed and extended tests. (Closed)

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

Description

Fixed and improved code for integral division. Fixed and extended tests. Arithmetic right shifting is *not* division in two's complement representation, only in one's complement. So we convert to one's complement, shift, and go back to two's complement. By permutating the last steps, one can get efficient branch-free code. This insight comes from the paleozoic era of computer science, see the paper from 1976: Guy Lewis Steele Jr.: "Arithmetic Shifting Considered Harmful" ftp://publications.ai.mit.edu/ai-publications/pdf/AIM-378.pdf This results in better and more correct code than our previous "neg/shift/neg" dance. LOG=y BUG=v8:3151 R=bmeurer@chromium.org Committed: https://code.google.com/p/v8/source/detail?r=19434

Patch Set 1 #

Patch Set 2 : x64 port #

Patch Set 3 : Rebased. Added ARM port. Fixes. #

Patch Set 4 : First (almost correct) A64 port. #

Patch Set 5 : A64 fixes and cleanup #

Total comments: 2
Unified diffs Side-by-side diffs Stats (+203 lines, -257 lines)
M src/a64/lithium-a64.h View 1 chunk +2 lines, -0 lines 0 comments
M src/a64/lithium-a64.cc View 1 chunk +1 line, -1 line 0 comments
M src/a64/lithium-codegen-a64.cc View 1 chunk +87 lines, -114 lines 1 comment
M src/arm/lithium-arm.cc View 1 chunk +1 line, -1 line 0 comments
M src/arm/lithium-codegen-arm.cc View 1 chunk +37 lines, -45 lines 1 comment
M src/ia32/lithium-codegen-ia32.cc View 1 chunk +29 lines, -44 lines 0 comments
M src/ia32/lithium-ia32.cc View 1 chunk +2 lines, -2 lines 0 comments
M src/x64/lithium-codegen-x64.cc View 1 chunk +30 lines, -47 lines 0 comments
M src/x64/lithium-x64.cc View 1 chunk +2 lines, -2 lines 0 comments
M test/mjsunit/shift-for-integer-div.js View 2 chunks +12 lines, -1 line 0 comments

Messages

Total messages: 3 (0 generated)
Sven Panne
6 years, 10 months ago (2014-02-18 09:20:43 UTC) #1
Benedikt Meurer
LGTM with nit. https://codereview.chromium.org/166793002/diff/100001/src/a64/lithium-codegen-a64.cc File src/a64/lithium-codegen-a64.cc (right): https://codereview.chromium.org/166793002/diff/100001/src/a64/lithium-codegen-a64.cc#newcode2622 src/a64/lithium-codegen-a64.cc:2622: __ Add(result, result, dividend); Please merge ...
6 years, 10 months ago (2014-02-18 09:43:30 UTC) #2
Sven Panne
6 years, 10 months ago (2014-02-18 10:45:38 UTC) #3
Message was sent while issue was closed.
Committed patchset #5 manually as r19434 (presubmit successful).

Powered by Google App Engine