Classes: Add basic support for properties

Classes: Add basic support for properties

WIP!

DO NOT SUBMIT

[arv (Not doing code reviews), 2014-10-17 22:00:47]

arv@chromium.org changed reviewers:
+ dslomov@chromium.org

[arv (Not doing code reviews), 2014-10-17 22:00:47]

I finally got some cycles over to work on this.

Is this on the right track?

I'm hittiong some asserts in bailout but I haven't investigated this yet.

Thanks

[Dmitry Lomov (no reviews), 2014-10-18 08:30:33]

On a right track, shaping nicely.

https://codereview.chromium.org/639123009/diff/1/src/full-codegen.cc
File src/full-codegen.cc (right):

https://codereview.chromium.org/639123009/diff/1/src/full-codegen.cc#newcode1571
src/full-codegen.cc:1571: EmitClassDefineProperties(lit);
nit: keep plugging context here - this is a result of  ClassLiteral evluation,
not of emitting class properties. The contract should be that
EmitClassDefineProperties has a class in result_register()

https://codereview.chromium.org/639123009/diff/1/src/ia32/full-codegen-ia32.cc
File src/ia32/full-codegen-ia32.cc (right):

https://codereview.chromium.org/639123009/diff/1/src/ia32/full-codegen-ia32.c...
src/ia32/full-codegen-ia32.cc:2424: __ mov(scratch, FieldOperand(eax,
JSFunction::kPrototypeOrInitialMapOffset));
Loading prototype is an operation that requires access check :(
So you need to call GetPrototype here, just like for super(...) call.
I guess extract that into a separate EmitGetPrototype.

https://codereview.chromium.org/639123009/diff/1/src/ia32/full-codegen-ia32.c...
src/ia32/full-codegen-ia32.cc:2446: __ CallRuntime(Runtime::kSetProperty, 4);
You should use StoreIC here (see EmitKeyedPropertyAssignment for how it is done)

[Dmitry Lomov (no reviews), 2014-10-18 09:04:59]

https://codereview.chromium.org/639123009/diff/1/src/ia32/full-codegen-ia32.cc
File src/ia32/full-codegen-ia32.cc (right):

https://codereview.chromium.org/639123009/diff/1/src/ia32/full-codegen-ia32.c...
src/ia32/full-codegen-ia32.cc:2446: __ CallRuntime(Runtime::kSetProperty, 4);
On 2014/10/18 08:30:33, Dmitry Lomov (chromium) wrote:
> You should use StoreIC here (see EmitKeyedPropertyAssignment for how it is
done)

s/Keyed/Named/ of course.
Maybe given how much runtime calling is going on here anyway, maybe it's not
worth it yet to install ICs here

[arv (Not doing code reviews), 2014-10-20 20:15:08]

Add failing test

[arv (Not doing code reviews), 2014-10-20 20:20:34]

Use define data property instead

[arv (Not doing code reviews), 2014-10-20 20:22:31]

https://codereview.chromium.org/639123009/diff/1/src/ia32/full-codegen-ia32.cc
File src/ia32/full-codegen-ia32.cc (right):

https://codereview.chromium.org/639123009/diff/1/src/ia32/full-codegen-ia32.c...
src/ia32/full-codegen-ia32.cc:2446: __ CallRuntime(Runtime::kSetProperty, 4);
On 2014/10/18 09:04:59, Dmitry Lomov (chromium) wrote:
> On 2014/10/18 08:30:33, Dmitry Lomov (chromium) wrote:
> > You should use StoreIC here (see EmitKeyedPropertyAssignment for how it is
> done)
> 
> s/Keyed/Named/ of course.
> Maybe given how much runtime calling is going on here anyway, maybe it's not
> worth it yet to install ICs here

I'm not sure that is correct? We are not doing assignment here but define.

[Dmitry Lomov (no reviews), 2014-10-21 06:17:05]

https://codereview.chromium.org/639123009/diff/40001/src/ia32/full-codegen-ia...
File src/ia32/full-codegen-ia32.cc (right):

https://codereview.chromium.org/639123009/diff/40001/src/ia32/full-codegen-ia...
src/ia32/full-codegen-ia32.cc:2446: __
CallRuntime(Runtime::kDefineDataPropertyUnchecked, 4);
Ah, that's better, now I understand :)
This one is indeed not equivalent to named store!

[arv (Not doing code reviews), 2014-10-21 15:19:14]

Testing CallStoreIC

[arv (Not doing code reviews), 2014-10-27 13:20:11]

Fixes issue with bailout id

[arv (Not doing code reviews), 2014-10-27 13:22:44]

Patchset #2 (id:20001) has been deleted

[arv (Not doing code reviews), 2014-10-27 13:22:58]

Patchset #3 (id:60001) has been deleted

[arv (Not doing code reviews), 2014-10-27 13:23:28]

PTAL

[Dmitry Lomov (no reviews), 2014-10-27 14:11:56]

On 2014/10/27 13:23:28, arv wrote:
> PTAL

ia32 looks good

[arv (Not doing code reviews), 2014-10-27 19:15:52]

Moved to https://codereview.chromium.org/680993003