Changed structure of accessor arguments passing to allow accessor

Changed structure of accessor arguments passing to allow accessor
callbacks to be called directly from native code.

[Christian Plesner Hansen, 2009-09-29 15:33:27]

This code is pending performance numbers -- it's expected to be more or less a
wash.  If it's not that needs to be looked into before I submit it.

[antonm, 2009-09-29 15:49:38]

Very cool

http://codereview.chromium.org/242050/diff/1/4
File src/arguments.h (right):

http://codereview.chromium.org/242050/diff/1/4#newcode73
Line 73: // Cursom arguments replicate a small segment of stack that can be
typo: Custom

http://codereview.chromium.org/242050/diff/1/9
File src/stub-cache.cc (right):

http://codereview.chromium.org/242050/diff/1/9#newcode792
Line 792: Object* LoadPropertyWithInterceptorOnly(Arguments args) {
Maybe add here and into other similar places NoAllocation stuff?

[Christian Plesner Hansen, 2009-09-29 16:38:45]

http://codereview.chromium.org/242050/diff/1/9
File src/stub-cache.cc (right):

http://codereview.chromium.org/242050/diff/1/9#newcode792
Line 792: Object* LoadPropertyWithInterceptorOnly(Arguments args) {
No need, a CustomArguments instance is GC safe.

[antonm, 2009-09-29 16:41:11]

http://codereview.chromium.org/242050/diff/1/9
File src/stub-cache.cc (right):

http://codereview.chromium.org/242050/diff/1/9#newcode792
Line 792: Object* LoadPropertyWithInterceptorOnly(Arguments args) {
On 2009/09/29 16:38:46, Christian Plesner Hansen wrote:
> No need, a CustomArguments instance is GC safe.
> 

They are, but locals like receiver_handle (which, btw, should be probably
renamed into just receiver) are not, correct?

[Christian Plesner Hansen, 2009-09-29 17:13:33]

http://codereview.chromium.org/242050/diff/1/9
File src/stub-cache.cc (right):

http://codereview.chromium.org/242050/diff/1/9#newcode792
Line 792: Object* LoadPropertyWithInterceptorOnly(Arguments args) {
> They are, but locals like receiver_handle (which, btw, should be probably
> renamed into just receiver) are not, correct?

Yes they should be renamed, good point.  In any case they are not used for
anything but creating the CustomArguments so it's safe, and we can't disallow
allocation because the getter might  allocate.

[antonm, 2009-09-29 17:22:20]

http://codereview.chromium.org/242050/diff/1/9
File src/stub-cache.cc (right):

http://codereview.chromium.org/242050/diff/1/9#newcode792
Line 792: Object* LoadPropertyWithInterceptorOnly(Arguments args) {
On 2009/09/29 17:13:33, Christian Plesner Hansen wrote:
> > They are, but locals like receiver_handle (which, btw, should be probably
> > renamed into just receiver) are not, correct?
> 
> Yes they should be renamed, good point.  In any case they are not used for
> anything but creating the CustomArguments so it's safe, and we can't disallow
> allocation because the getter might  allocate.
> 

Good point about possibility of GC.  Could we get rid of locals altogether then?
 E.g. it looks like receiver, holder and data are not used except for CustomArgs
ctor, so maybe just drop locals and use CustomArguments args(args[4],
JSObject::cast(args[1]), ...)?

[Christian Plesner Hansen, 2009-09-29 17:32:25]

http://codereview.chromium.org/242050/diff/1/9
File src/stub-cache.cc (right):

http://codereview.chromium.org/242050/diff/1/9#newcode792
Line 792: Object* LoadPropertyWithInterceptorOnly(Arguments args) {
The reason I kept them was to make it obvious how the argument indices mapped to
receiver, holder, etc.  But I can move them into a scope closer around the call
to the getter.

[Christian Plesner Hansen, 2009-09-30 07:53:04]

http://codereview.chromium.org/242050/diff/1/4
File src/arguments.h (right):

http://codereview.chromium.org/242050/diff/1/4#newcode73
Line 73: // Cursom arguments replicate a small segment of stack that can be
Fixed.

http://codereview.chromium.org/242050/diff/1/9
File src/stub-cache.cc (right):

http://codereview.chromium.org/242050/diff/1/9#newcode792
Line 792: Object* LoadPropertyWithInterceptorOnly(Arguments args) {
Done

[Erik Corry, 2009-09-30 11:41:33]

LGTM

http://codereview.chromium.org/242050/diff/14/17
File src/arguments.h (right):

http://codereview.chromium.org/242050/diff/14/17#newcode82
Line 82: operator[](0) = self;
Funky syntax!

http://codereview.chromium.org/242050/diff/14/22
File src/stub-cache.cc (right):

http://codereview.chromium.org/242050/diff/14/22#newcode743
Line 743: // NOTE: If we can align the structure of an AccessorInfo with the
This comment looks outdated to me.

[iposva, 2009-09-30 22:19:05]

Drive by comments!

-Ivan

http://codereview.chromium.org/242050/diff/5002/7003
File src/arguments.h (right):

http://codereview.chromium.org/242050/diff/5002/7003#newcode66
Line 66: 
Presubmit Check: Fail

http://codereview.chromium.org/242050/diff/5002/7003#newcode75
Line 75: // Cursom arguments replicate a small segment of stack that can be
Cursom -> Custom

http://codereview.chromium.org/242050/diff/5002/7003#newcode79
Line 79: public:
Presubmit Check: Fail

http://codereview.chromium.org/242050/diff/5002/7003#newcode90
Line 90: private:
Presubmit Check: Fail

http://codereview.chromium.org/242050/diff/5002/7007
File src/objects.h (right):

http://codereview.chromium.org/242050/diff/5002/7007#newcode4223
Line 4223: // on gc.  It provides two ways for the gc to update instances,
either
gc -> GC

http://codereview.chromium.org/242050/diff/5002/7007#newcode4226
Line 4226: public:
Presubmit Check: Fail

http://codereview.chromium.org/242050/diff/5002/7007#newcode4228
Line 4228: virtual ~Relocatable() { ASSERT_EQ(top_, this); top_ = prev_; }
Presubmit Check: Fail

http://codereview.chromium.org/242050/diff/5002/7007#newcode4239
Line 4239: private:
Presubmit Check: Fail