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

Issue 458813002: Prototype implementation of GET_OWN_PROPERTY intrinsic. (Closed)

Created:
6 years, 4 months ago by Dmitry Lomov (no reviews)
Modified:
6 years, 4 months ago
Reviewers:
CC:
v8-dev, Toon Verwaest, rossberg
Project:
v8
Visibility:
Public.

Description

Prototype implementation of GET_OWN_PROPERTY intrinsic. This is an exploration of changes necessary to support optimizable GET_OWN_PROPERTY intrinsic. The overview: 1. Parser expands %GET_OWN_PROPERTY(o, p) into Property AST node with a special 'own' mark 2. LoadIC and KeyedLoadIC acquire an extra IC state, PropertyLookupMode 3. Quite non-trivial amount of plumbing ensues but eventually LoadIC_Miss does the right lookup 4. A lot of checks here and there to avoid normal/own property lookup interference. Only ia32 of course. No turbo-fan support (but crankshaft works!) All tests pass, but of course this needs more testing. Overall the whole thing looks intrusive and frankly feels like a bug farm :) However the ability to have optimizable access to private state on objects sounds like a good thing to have. Ideas for simplification are welcome of course. Let me know what you think. P.S. At least the part of the patch that changes Strings to Names is useful in its own right - I'll prep a separate patch with just those changes

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+437 lines, -105 lines) Patch
M src/accessors.h View 1 chunk +1 line, -1 line 0 comments Download
M src/accessors.cc View 3 chunks +5 lines, -5 lines 0 comments Download
M src/ast.h View 4 chunks +12 lines, -1 line 0 comments Download
M src/builtins.h View 2 chunks +2 lines, -2 lines 0 comments Download
M src/builtins.cc View 2 chunks +13 lines, -2 lines 0 comments Download
M src/code-stubs.h View 2 chunks +11 lines, -2 lines 0 comments Download
M src/code-stubs.cc View 1 chunk +1 line, -1 line 0 comments Download
M src/compiler/js-generic-lowering.cc View 1 chunk +2 lines, -1 line 0 comments Download
M src/full-codegen.h View 1 chunk +3 lines, -0 lines 0 comments Download
M src/full-codegen.cc View 1 chunk +13 lines, -1 line 0 comments Download
M src/hydrogen.cc View 2 chunks +4 lines, -2 lines 0 comments Download
M src/hydrogen-instructions.h View 8 chunks +15 lines, -8 lines 0 comments Download
M src/ia32/full-codegen-ia32.cc View 2 chunks +24 lines, -7 lines 0 comments Download
M src/ia32/ic-ia32.cc View 5 chunks +38 lines, -13 lines 0 comments Download
M src/ia32/lithium-codegen-ia32.cc View 3 chunks +17 lines, -3 lines 0 comments Download
M src/ia32/lithium-ia32.h View 2 chunks +2 lines, -0 lines 0 comments Download
M src/ic.h View 8 chunks +67 lines, -24 lines 0 comments Download
M src/ic.cc View 21 chunks +60 lines, -29 lines 0 comments Download
M src/parser.cc View 1 chunk +5 lines, -0 lines 0 comments Download
M src/runtime.h View 2 chunks +3 lines, -0 lines 0 comments Download
M src/runtime.cc View 3 chunks +47 lines, -0 lines 0 comments Download
M src/stub-cache.h View 2 chunks +8 lines, -0 lines 0 comments Download
M src/stub-cache.cc View 4 chunks +54 lines, -3 lines 0 comments Download
A test/mjsunit/getownprivateproperty.js View 1 chunk +30 lines, -0 lines 0 comments Download

Messages

Total messages: 3 (0 generated)
Dmitry Lomov (no reviews)
Toon, Andreas, please take a look.
6 years, 4 months ago (2014-08-10 14:48:44 UTC) #1
Dmitry Lomov (no reviews)
On 2014/08/10 14:48:44, Dmitry Lomov (chromium) wrote: > Toon, Andreas, please take a look. After ...
6 years, 4 months ago (2014-08-11 10:14:36 UTC) #2
Dmitry Lomov (no reviews)
6 years, 4 months ago (2014-08-11 16:04:57 UTC) #3
On 2014/08/11 10:14:36, Dmitry Lomov (chromium) wrote:
> On 2014/08/10 14:48:44, Dmitry Lomov (chromium) wrote:
> > Toon, Andreas, please take a look.
> 
> After some discussion with Toon we figured that maybe a less intrusive thing
to
> try is to make 'own' a property of a symbol not a lookup (all lookups for a
> particular symbol are own property lookups). I'll try to go down that rabbit
> hole to see whether it is indeed less deep than this one.

Ok that rabbit hole is way shallower: https://codereview.chromium.org/464473002/

Powered by Google App Engine
This is Rietveld 408576698