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

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 167683002: Simplify type argument instantiation cache lookup by introducing an array (Closed)

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

Description

Simplify type argument instantiation cache lookup by introducing an array of 1 zero Smi. Was replaced by https://codereview.chromium.org//169223005

Patch Set 1 #

Total comments: 4
Unified diffs Side-by-side diffs Delta from patch set Stats (+94 lines, -82 lines) Patch
runtime/vm/intermediate_language_arm.cc View 1 chunk +2 lines, -4 lines 0 comments Download
runtime/vm/intermediate_language_ia32.cc View 1 chunk +2 lines, -4 lines 2 comments Download
runtime/vm/intermediate_language_mips.cc View 1 chunk +2 lines, -4 lines 0 comments Download
runtime/vm/intermediate_language_x64.cc View 1 chunk +2 lines, -4 lines 0 comments Download
runtime/vm/object.h View 2 chunks +5 lines, -0 lines 0 comments Download
runtime/vm/object.cc View 8 chunks +34 lines, -11 lines 2 comments Download
runtime/vm/object_test.cc View 1 chunk +4 lines, -0 lines 0 comments Download
runtime/vm/raw_object_snapshot.cc View 1 chunk +1 line, -2 lines 0 comments Download
runtime/vm/snapshot.cc View 2 chunks +9 lines, -0 lines 0 comments Download
runtime/vm/snapshot_ids.h View 1 chunk +1 line, -0 lines 0 comments Download
runtime/vm/stub_code_arm.cc View 3 chunks +8 lines, -13 lines 0 comments Download
runtime/vm/stub_code_ia32.cc View 3 chunks +8 lines, -13 lines 0 comments Download
runtime/vm/stub_code_mips.cc View 3 chunks +8 lines, -14 lines 0 comments Download
runtime/vm/stub_code_x64.cc View 3 chunks +8 lines, -13 lines 0 comments Download

Messages

Total messages: 4 (1 generated)
regis
6 years, 10 months ago (2014-02-14 21:12:59 UTC) #1
Florian Schneider
LGTM. https://codereview.chromium.org/167683002/diff/1/runtime/vm/intermediate_language_ia32.cc File runtime/vm/intermediate_language_ia32.cc (right): https://codereview.chromium.org/167683002/diff/1/runtime/vm/intermediate_language_ia32.cc#newcode2199 runtime/vm/intermediate_language_ia32.cc:2199: __ addl(EDI, Immediate(2 * kWordSize)); You could save ...
6 years, 10 months ago (2014-02-17 10:09:58 UTC) #2
regis
6 years, 10 months ago (2014-02-18 19:29:29 UTC) #3 
Thanks!

https://codereview.chromium.org/167683002/diff/1/runtime/vm/intermediate_lang...
File runtime/vm/intermediate_language_ia32.cc (right):

https://codereview.chromium.org/167683002/diff/1/runtime/vm/intermediate_lang...
runtime/vm/intermediate_language_ia32.cc:2199: __ addl(EDI, Immediate(2 *
kWordSize));
On 2014/02/17 10:09:59, Florian Schneider wrote:
> You could save one branch instruction by reordering the code like this:
> 
> __ addl(EDI, Immediate(2 * kWordSize));
> __ cmpl(EDX, Immediate(Smi::RawValue(StubCode::kNoInstantiator)));
> __ j(NOT_EQUAL, &loop);
> 
> 
> This should work on all platforms.

Done on all platforms.

https://codereview.chromium.org/167683002/diff/1/runtime/vm/object.cc
File runtime/vm/object.cc (right):

https://codereview.chromium.org/167683002/diff/1/runtime/vm/object.cc#newcode...
runtime/vm/object.cc:4297: ASSERT(StubCode::kNoInstantiator == 0);
On 2014/02/17 10:09:59, Florian Schneider wrote:
> That can be make a COMPILE_ASSERT.

I had never noticed this assert flavor before. Done.

Powered by Google App Engine
This is Rietveld 408576698