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

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 562333004: Recognize Bigint setters and getters. This code will be removed when/if we change the fields to be … (Closed)

Created:
6 years, 3 months ago by srdjan
Modified:
6 years, 3 months ago
CC:
reviews_dartlang.org, vm-dev_dartlang.org, regis
Visibility:
Public.

Description

Recognize Bigint setters and getters. This code will be removed when/if we change the fields to be Dart instead fo native fields. R=regis@google.com Committed: https://code.google.com/p/dart/source/detail?r=40206

Patch Set 1 #

Patch Set 2 : #

Total comments: 5

Patch Set 3 : #

Patch Set 4 : #

Unified diffs Side-by-side diffs Delta from patch set Stats (+93 lines, -109 lines) Patch
M runtime/vm/flow_graph_builder.cc View 1 2 3 1 chunk +35 lines, -0 lines 0 comments Download
M runtime/vm/flow_graph_optimizer.cc View 1 2 3 1 chunk +51 lines, -0 lines 0 comments Download
M runtime/vm/intrinsifier_arm.cc View 1 2 3 3 chunks +0 lines, -21 lines 0 comments Download
M runtime/vm/intrinsifier_arm64.cc View 1 2 3 3 chunks +0 lines, -21 lines 0 comments Download
M runtime/vm/intrinsifier_ia32.cc View 1 2 3 3 chunks +0 lines, -21 lines 0 comments Download
M runtime/vm/intrinsifier_mips.cc View 1 2 3 3 chunks +0 lines, -21 lines 0 comments Download
M runtime/vm/intrinsifier_x64.cc View 3 chunks +0 lines, -21 lines 0 comments Download
M runtime/vm/method_recognizer.h View 2 chunks +7 lines, -4 lines 0 comments Download

Messages

Total messages: 9 (2 generated)
srdjan
6 years, 3 months ago (2014-09-11 22:24:06 UTC) #2
regis
LGTM https://codereview.chromium.org/562333004/diff/20001/runtime/vm/flow_graph_builder.cc File runtime/vm/flow_graph_builder.cc (right): https://codereview.chromium.org/562333004/diff/20001/runtime/vm/flow_graph_builder.cc#newcode3286 runtime/vm/flow_graph_builder.cc:3286: load->set_result_cid(kDynamicCid); Why can't you do it now?
6 years, 3 months ago (2014-09-11 22:33:30 UTC) #4
srdjan
https://codereview.chromium.org/562333004/diff/20001/runtime/vm/flow_graph_builder.cc File runtime/vm/flow_graph_builder.cc (right): https://codereview.chromium.org/562333004/diff/20001/runtime/vm/flow_graph_builder.cc#newcode3286 runtime/vm/flow_graph_builder.cc:3286: load->set_result_cid(kDynamicCid); On 2014/09/11 22:33:30, regis wrote: > Why can't ...
6 years, 3 months ago (2014-09-11 22:44:21 UTC) #5
Florian Schneider
dbc: https://codereview.chromium.org/562333004/diff/20001/runtime/vm/flow_graph_builder.cc File runtime/vm/flow_graph_builder.cc (right): https://codereview.chromium.org/562333004/diff/20001/runtime/vm/flow_graph_builder.cc#newcode3286 runtime/vm/flow_graph_builder.cc:3286: load->set_result_cid(kDynamicCid); On 2014/09/11 22:44:20, srdjan wrote: > On ...
6 years, 3 months ago (2014-09-12 09:17:39 UTC) #6
srdjan
https://codereview.chromium.org/562333004/diff/20001/runtime/vm/flow_graph_builder.cc File runtime/vm/flow_graph_builder.cc (right): https://codereview.chromium.org/562333004/diff/20001/runtime/vm/flow_graph_builder.cc#newcode3286 runtime/vm/flow_graph_builder.cc:3286: load->set_result_cid(kDynamicCid); On 2014/09/12 09:17:39, Florian Schneider wrote: > On ...
6 years, 3 months ago (2014-09-12 15:29:35 UTC) #7
srdjan
Committed patchset #4 (id:60001) manually as r40206 (presubmit successful).
6 years, 3 months ago (2014-09-12 15:45:46 UTC) #8
srdjan
6 years, 3 months ago (2014-09-18 16:12:55 UTC) #9
Message was sent while issue was closed. 
https://codereview.chromium.org/562333004/diff/20001/runtime/vm/flow_graph_bu...
File runtime/vm/flow_graph_builder.cc (right):

https://codereview.chromium.org/562333004/diff/20001/runtime/vm/flow_graph_bu...
runtime/vm/flow_graph_builder.cc:3286: load->set_result_cid(kDynamicCid);
On 2014/09/12 15:29:35, srdjan wrote:
> On 2014/09/12 09:17:39, Florian Schneider wrote:
> > On 2014/09/11 22:44:20, srdjan wrote:
> > > On 2014/09/11 22:33:30, regis wrote:
> > > > Why can't you do it now?
> > > 
> > > I must keep the type (passed to LoadFieldInstr) and the cid in sync. There
> is
> > no
> > > preallocated Type for Uint32Array.
> > 
> > I think it should just work to set result_cid, but leave the AbstractType
> > dynamic. We create CompileType just from cids also in the type propagator
> > (CompileType::FromCid) e.g. after a CheckClassInstr.
> 
> Currently it throws an NPE. Will investigate in next CL.

It throws an NPE because it is sometimes NULL and not always Uint32array. Will
modify libraries to guarantee that it is always Uint32Array

Powered by Google App Engine
This is Rietveld 408576698