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

Issue 353513002: Use range information for optimizing integer boxing and fix bug in range analysis. (Closed)

Created:
6 years, 6 months ago by Florian Schneider
Modified:
6 years, 6 months ago
CC:
reviews_dartlang.org, vm-dev_dartlang.org
Visibility:
Public.

Description

Use range information for optimizing integer boxing and fix bug in range analysis. 1. When the input to a BoxInteger operation is in smi-range, we can eliminate the range check and just perform a smi-tag operation on the input. 2. There was a bug in checking for smi-overflow for range boundaries: Calling Smi::IsValid with a int64_t argument resulted in silent truncation of the input and therefore a wrong result. Compiling with -Wconversion would have caught this, but currently we cannot compile with this flag because of too many broken places. Instead, I removed Smi::IsValid64 and created one variant Smi::IsValid that is specialized for the input type with a template parameter. This way calling Smi::IsValid is always safe and will never result in silent alteration of the input argument. R=vegorov@google.com Committed: https://code.google.com/p/dart/source/detail?r=37657

Patch Set 1 #

Total comments: 6

Patch Set 2 : #

Patch Set 3 : #

Unified diffs Side-by-side diffs Delta from patch set Stats (+136 lines, -33 lines) Patch
M runtime/lib/integers.cc View 1 2 1 chunk +1 line, -1 line 0 comments Download
M runtime/vm/dart_api_impl.cc View 1 2 2 chunks +2 lines, -2 lines 0 comments Download
M runtime/vm/dart_api_message.cc View 1 2 4 chunks +4 lines, -4 lines 0 comments Download
M runtime/vm/deferred_objects.cc View 1 2 1 chunk +1 line, -1 line 0 comments Download
M runtime/vm/deopt_instructions.cc View 1 2 3 chunks +3 lines, -3 lines 0 comments Download
M runtime/vm/flow_graph_type_propagator.cc View 1 2 1 chunk +8 lines, -0 lines 0 comments Download
M runtime/vm/intermediate_language.h View 1 2 5 chunks +12 lines, -1 line 0 comments Download
M runtime/vm/intermediate_language.cc View 1 2 4 chunks +51 lines, -3 lines 0 comments Download
M runtime/vm/intermediate_language_arm.cc View 1 2 2 chunks +16 lines, -3 lines 0 comments Download
M runtime/vm/intermediate_language_ia32.cc View 1 2 2 chunks +16 lines, -3 lines 0 comments Download
M runtime/vm/object.h View 1 2 2 chunks +12 lines, -5 lines 0 comments Download
M runtime/vm/object.cc View 1 2 4 chunks +4 lines, -4 lines 0 comments Download
M runtime/vm/object_test.cc View 1 2 1 chunk +5 lines, -2 lines 0 comments Download
M runtime/vm/service.cc View 1 2 1 chunk +1 line, -1 line 0 comments Download

Messages

Total messages: 4 (0 generated)
Florian Schneider
6 years, 6 months ago (2014-06-23 15:22:51 UTC) #1
Vyacheslav Egorov (Google)
lgtm https://codereview.chromium.org/353513002/diff/1/runtime/vm/intermediate_language.cc File runtime/vm/intermediate_language.cc (right): https://codereview.chromium.org/353513002/diff/1/runtime/vm/intermediate_language.cc#newcode3292 runtime/vm/intermediate_language.cc:3292: one more empty line https://codereview.chromium.org/353513002/diff/1/runtime/vm/object.h File runtime/vm/object.h (right): ...
6 years, 6 months ago (2014-06-24 12:18:12 UTC) #2
Florian Schneider
https://codereview.chromium.org/353513002/diff/1/runtime/vm/intermediate_language.cc File runtime/vm/intermediate_language.cc (right): https://codereview.chromium.org/353513002/diff/1/runtime/vm/intermediate_language.cc#newcode3292 runtime/vm/intermediate_language.cc:3292: On 2014/06/24 12:18:12, Vyacheslav Egorov (Google) wrote: > one ...
6 years, 6 months ago (2014-06-24 12:35:06 UTC) #3
Florian Schneider
6 years, 6 months ago (2014-06-24 13:42:01 UTC) #4
Message was sent while issue was closed.
Committed patchset #3 manually as r37657 (presubmit successful).

Powered by Google App Engine
This is Rietveld 408576698