Rework CallApi*Stubs.

Rework CallApi*Stubs.

- Eliminate stubs with a variable number of arguments.
  (That only worked due to their very limited use. These
   stubs' interface descriptors were basically lying
   about their number of args, which will fail when used
   generically.)
- Fix all CallApi*Stubs' interface descriptors to no
  longer lie about their arguments.
- Unify CallApi*Stub, for * in Function, Accessor,
  FunctionWithFixedArgs.
  (Since these are now all doing the same thing.)
- Rename the unified stub (and interface descriptors) to
  *ApiCallback*, since that's really what they're doing.
- Refuse inlining an API callback if its number of
  parameters exceeds the supported number of args.

BUG=

Committed: https://crrev.com/d238b953a474272c0e3ea22ef6a9b63fa9729340
Cr-Commit-Position: refs/heads/master@{#34614}

Committed: https://crrev.com/5096492f1b0460b2644c7f79f9ccc5881fc65c50
Cr-Commit-Position: refs/heads/master@{#34627}

[vogelheim, 2016-03-01 16:31:46]

Description was changed from

==========
Rework CallApi*Stubs.

- Eliminate stubs with a variable number of arguments.
  (That only worked due to their very limited use. These stubs' interface
   descriptors were basically lying about their number of args, which will
   fail when used generically.)
- Fix all CallApi*Stubs' interface descriptors to no longer lie about
  their arguments.
- Unify CallApi*Stub, for * in Function, Accessor, FunctionWithFixedArgs.
  (Since these are now all doing the same thing.)
- Rename the unified stub (and interface descriptors) to *ApiCallback*,
  since that's really what they're doing.

BUG=
==========

to

==========
Rework CallApi*Stubs.

- Eliminate stubs with a variable number of arguments.
  (That only worked due to their very limited use. These
   stubs' interface descriptors were basically lying
   about their number of args, which will fail when used
   generically.)
- Fix all CallApi*Stubs' interface descriptors to no
  longer lie about their arguments.
- Unify CallApi*Stub, for * in Function, Accessor, 
  FunctionWithFixedArgs.
  (Since these are now all doing the same thing.)
- Rename the unified stub (and interface descriptors) to 
  *ApiCallback*, since that's really what they're doing.
- Refuse inlining an API callback if its number of 
  parameters exceeds the supported number of args.

BUG=
==========

[vogelheim, 2016-03-01 16:56:29]

Patchset #1 (id:1) has been deleted

[vogelheim, 2016-03-01 17:00:49]

vogelheim@chromium.org changed reviewers:
+ danno@chromium.org

[vogelheim, 2016-03-01 17:00:49]

Please have a look.

This somehow became more elaborate than I had hoped; but at least all files
except interface-descriptors.* have more lines deleted than added. :)

If this looks generally good to you, I'll also change the platform specific
support for non-x64 architectures in the same CL.

[danno, 2016-03-04 18:07:43]

https://codereview.chromium.org/1748123003/diff/20001/src/interface-descripto...
File src/interface-descriptors.cc (right):

https://codereview.chromium.org/1748123003/diff/20001/src/interface-descripto...
src/interface-descriptors.cc:515: return
ApiCallbackWith7ArgsDescriptor(isolate);
Shouldn't you have a default case that is also unreachable? Don't know what the
compiler would do in the case that you pass "8".

https://codereview.chromium.org/1748123003/diff/20001/src/interface-descripto...
File src/interface-descriptors.h (right):

https://codereview.chromium.org/1748123003/diff/20001/src/interface-descripto...
src/interface-descriptors.h:723: template <int argc>
Is this template magic really necessary? Can you not just create a new macro
DECLARE_API_CALLBACK_WITH_ARGS_DESCRIPTOR with a integer argument that does that
same? It seems a little less magic.

[vogelheim, 2016-03-08 12:59:27]

ptal

https://codereview.chromium.org/1748123003/diff/20001/src/interface-descripto...
File src/interface-descriptors.cc (right):

https://codereview.chromium.org/1748123003/diff/20001/src/interface-descripto...
src/interface-descriptors.cc:515: return
ApiCallbackWith7ArgsDescriptor(isolate);
On 2016/03/04 18:07:42, danno wrote:
> Shouldn't you have a default case that is also unreachable? Don't know what
the
> compiler would do in the case that you pass "8".

Done.

I'm honestly a bit confused at this feedback, though. With argc == 8, the switch
should have just fall through and reach the UNREACHABLE.

https://codereview.chromium.org/1748123003/diff/20001/src/interface-descripto...
File src/interface-descriptors.h (right):

https://codereview.chromium.org/1748123003/diff/20001/src/interface-descripto...
src/interface-descriptors.h:723: template <int argc>
On 2016/03/04 18:07:43, danno wrote:
> Is this template magic really necessary? Can you not just create a new macro
> DECLARE_API_CALLBACK_WITH_ARGS_DESCRIPTOR with a integer argument that does
that
> same? It seems a little less magic.

Removed, and replaced with DECLARE_DESCRIPTOR_WITH_BASE_AND_FUNCTION_TYPE_ARG.

This does look nicer.

[danno, 2016-03-09 10:23:38]

lgtm

https://codereview.chromium.org/1748123003/diff/20001/src/interface-descripto...
File src/interface-descriptors.cc (right):

https://codereview.chromium.org/1748123003/diff/20001/src/interface-descripto...
src/interface-descriptors.cc:515: return
ApiCallbackWith7ArgsDescriptor(isolate);
On 2016/03/08 12:59:27, vogelheim wrote:
> On 2016/03/04 18:07:42, danno wrote:
> > Shouldn't you have a default case that is also unreachable? Don't know what
> the
> > compiler would do in the case that you pass "8".
> 
> Done.
> 
> I'm honestly a bit confused at this feedback, though. With argc == 8, the
switch
> should have just fall through and reach the UNREACHABLE.

I looked at the C/C++ spec, and you're right. Without a default, it just falls
through, so feel free to go back to what you had.

https://codereview.chromium.org/1748123003/diff/100001/src/compiler/fast-acce...
File src/compiler/fast-accessor-assembler.cc (right):

https://codereview.chromium.org/1748123003/diff/100001/src/compiler/fast-acce...
src/compiler/fast-accessor-assembler.cc:197: assembler_->UndefinedConstant()};
It might be slightly more efficient to pass Smi(0) here.

[vogelheim, 2016-03-09 11:09:35]

https://codereview.chromium.org/1748123003/diff/100001/src/compiler/fast-acce...
File src/compiler/fast-accessor-assembler.cc (right):

https://codereview.chromium.org/1748123003/diff/100001/src/compiler/fast-acce...
src/compiler/fast-accessor-assembler.cc:197: assembler_->UndefinedConstant()};
On 2016/03/09 10:23:38, danno wrote:
> It might be slightly more efficient to pass Smi(0) here.

Ack. But: The point of this CL is to enable moving FastAccessorAssembler out of
compiler/ and from CallN to CallStub. That will also give me the chance to fix
the Context properly - it shouldn't be 0 / Undefined anyhow - so this will be
removed anyhow in the follow-on CL.

[vogelheim, 2016-03-09 11:09:54]

The CQ bit was checked by vogelheim@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-03-09 11:10:03]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1748123003/100001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1748123003/100001

[commit-bot: I haz the power, 2016-03-09 11:11:50]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-03-09 11:11:50]

Dry run: This issue passed the CQ dry run.

[vogelheim, 2016-03-09 11:48:27]

The CQ bit was checked by vogelheim@chromium.org

[commit-bot: I haz the power, 2016-03-09 11:48:37]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1748123003/100001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1748123003/100001

[commit-bot: I haz the power, 2016-03-09 11:51:27]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-03-09 11:51:28]

Try jobs failed on following builders:
  v8_presubmit on tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_presubmit/builds/12081)

[Jakob Kummerow, 2016-03-09 12:11:06]

jkummerow@chromium.org changed reviewers:
+ jkummerow@chromium.org

[Jakob Kummerow, 2016-03-09 12:11:06]

src/ic/* LGTM.

[Michael Starzinger, 2016-03-09 12:15:42]

mstarzinger@chromium.org changed reviewers:
+ mstarzinger@chromium.org

[Michael Starzinger, 2016-03-09 12:15:43]

LGTM stamp on "compiler", didn't look at the rest.

[vogelheim, 2016-03-09 12:16:12]

The CQ bit was checked by vogelheim@chromium.org

[commit-bot: I haz the power, 2016-03-09 12:16:24]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1748123003/100001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1748123003/100001

[commit-bot: I haz the power, 2016-03-09 12:18:08]

Description was changed from

==========
Rework CallApi*Stubs.

- Eliminate stubs with a variable number of arguments.
  (That only worked due to their very limited use. These
   stubs' interface descriptors were basically lying
   about their number of args, which will fail when used
   generically.)
- Fix all CallApi*Stubs' interface descriptors to no
  longer lie about their arguments.
- Unify CallApi*Stub, for * in Function, Accessor, 
  FunctionWithFixedArgs.
  (Since these are now all doing the same thing.)
- Rename the unified stub (and interface descriptors) to 
  *ApiCallback*, since that's really what they're doing.
- Refuse inlining an API callback if its number of 
  parameters exceeds the supported number of args.

BUG=
==========

to

==========
Rework CallApi*Stubs.

- Eliminate stubs with a variable number of arguments.
  (That only worked due to their very limited use. These
   stubs' interface descriptors were basically lying
   about their number of args, which will fail when used
   generically.)
- Fix all CallApi*Stubs' interface descriptors to no
  longer lie about their arguments.
- Unify CallApi*Stub, for * in Function, Accessor, 
  FunctionWithFixedArgs.
  (Since these are now all doing the same thing.)
- Rename the unified stub (and interface descriptors) to 
  *ApiCallback*, since that's really what they're doing.
- Refuse inlining an API callback if its number of 
  parameters exceeds the supported number of args.

BUG=
==========

[commit-bot: I haz the power, 2016-03-09 12:18:09]

Committed patchset #5 (id:100001)

[commit-bot: I haz the power, 2016-03-09 12:18:21]

Description was changed from

==========
Rework CallApi*Stubs.

- Eliminate stubs with a variable number of arguments.
  (That only worked due to their very limited use. These
   stubs' interface descriptors were basically lying
   about their number of args, which will fail when used
   generically.)
- Fix all CallApi*Stubs' interface descriptors to no
  longer lie about their arguments.
- Unify CallApi*Stub, for * in Function, Accessor, 
  FunctionWithFixedArgs.
  (Since these are now all doing the same thing.)
- Rename the unified stub (and interface descriptors) to 
  *ApiCallback*, since that's really what they're doing.
- Refuse inlining an API callback if its number of 
  parameters exceeds the supported number of args.

BUG=
==========

to

==========
Rework CallApi*Stubs.

- Eliminate stubs with a variable number of arguments.
  (That only worked due to their very limited use. These
   stubs' interface descriptors were basically lying
   about their number of args, which will fail when used
   generically.)
- Fix all CallApi*Stubs' interface descriptors to no
  longer lie about their arguments.
- Unify CallApi*Stub, for * in Function, Accessor,
  FunctionWithFixedArgs.
  (Since these are now all doing the same thing.)
- Rename the unified stub (and interface descriptors) to
  *ApiCallback*, since that's really what they're doing.
- Refuse inlining an API callback if its number of
  parameters exceeds the supported number of args.

BUG=

Committed: https://crrev.com/d238b953a474272c0e3ea22ef6a9b63fa9729340
Cr-Commit-Position: refs/heads/master@{#34614}
==========

[commit-bot: I haz the power, 2016-03-09 12:18:22]

Patchset 5 (id:??) landed as
https://crrev.com/d238b953a474272c0e3ea22ef6a9b63fa9729340
Cr-Commit-Position: refs/heads/master@{#34614}

[vogelheim, 2016-03-09 14:58:23]

A revert of this CL (patchset #5 id:100001) has been created in
https://codereview.chromium.org/1775933005/ by vogelheim@chromium.org.

The reason for reverting is: Breaks Chromium..

[vogelheim, 2016-03-09 15:49:21]

Description was changed from

==========
Rework CallApi*Stubs.

- Eliminate stubs with a variable number of arguments.
  (That only worked due to their very limited use. These
   stubs' interface descriptors were basically lying
   about their number of args, which will fail when used
   generically.)
- Fix all CallApi*Stubs' interface descriptors to no
  longer lie about their arguments.
- Unify CallApi*Stub, for * in Function, Accessor,
  FunctionWithFixedArgs.
  (Since these are now all doing the same thing.)
- Rename the unified stub (and interface descriptors) to
  *ApiCallback*, since that's really what they're doing.
- Refuse inlining an API callback if its number of
  parameters exceeds the supported number of args.

BUG=

Committed: https://crrev.com/d238b953a474272c0e3ea22ef6a9b63fa9729340
Cr-Commit-Position: refs/heads/master@{#34614}
==========

to

==========
Rework CallApi*Stubs.

- Eliminate stubs with a variable number of arguments.
  (That only worked due to their very limited use. These
   stubs' interface descriptors were basically lying
   about their number of args, which will fail when used
   generically.)
- Fix all CallApi*Stubs' interface descriptors to no
  longer lie about their arguments.
- Unify CallApi*Stub, for * in Function, Accessor,
  FunctionWithFixedArgs.
  (Since these are now all doing the same thing.)
- Rename the unified stub (and interface descriptors) to
  *ApiCallback*, since that's really what they're doing.
- Refuse inlining an API callback if its number of
  parameters exceeds the supported number of args.

BUG=

Committed: https://crrev.com/d238b953a474272c0e3ea22ef6a9b63fa9729340
Cr-Commit-Position: refs/heads/master@{#34614}
==========

[vogelheim, 2016-03-09 15:49:37]

The CQ bit was checked by vogelheim@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-03-09 15:49:48]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1748123003/100001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1748123003/100001

[commit-bot: I haz the power, 2016-03-09 16:10:18]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-03-09 16:10:20]

Dry run: This issue passed the CQ dry run.

[vogelheim, 2016-03-09 16:10:35]

The CQ bit was checked by vogelheim@chromium.org

[commit-bot: I haz the power, 2016-03-09 16:10:47]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1748123003/100001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1748123003/100001

[commit-bot: I haz the power, 2016-03-09 16:18:33]

Description was changed from

==========
Rework CallApi*Stubs.

- Eliminate stubs with a variable number of arguments.
  (That only worked due to their very limited use. These
   stubs' interface descriptors were basically lying
   about their number of args, which will fail when used
   generically.)
- Fix all CallApi*Stubs' interface descriptors to no
  longer lie about their arguments.
- Unify CallApi*Stub, for * in Function, Accessor,
  FunctionWithFixedArgs.
  (Since these are now all doing the same thing.)
- Rename the unified stub (and interface descriptors) to
  *ApiCallback*, since that's really what they're doing.
- Refuse inlining an API callback if its number of
  parameters exceeds the supported number of args.

BUG=

Committed: https://crrev.com/d238b953a474272c0e3ea22ef6a9b63fa9729340
Cr-Commit-Position: refs/heads/master@{#34614}
==========

to

==========
Rework CallApi*Stubs.

- Eliminate stubs with a variable number of arguments.
  (That only worked due to their very limited use. These
   stubs' interface descriptors were basically lying
   about their number of args, which will fail when used
   generically.)
- Fix all CallApi*Stubs' interface descriptors to no
  longer lie about their arguments.
- Unify CallApi*Stub, for * in Function, Accessor,
  FunctionWithFixedArgs.
  (Since these are now all doing the same thing.)
- Rename the unified stub (and interface descriptors) to
  *ApiCallback*, since that's really what they're doing.
- Refuse inlining an API callback if its number of
  parameters exceeds the supported number of args.

BUG=

Committed: https://crrev.com/d238b953a474272c0e3ea22ef6a9b63fa9729340
Cr-Commit-Position: refs/heads/master@{#34614}
==========

[commit-bot: I haz the power, 2016-03-09 16:18:34]

Committed patchset #5 (id:100001)

[commit-bot: I haz the power, 2016-03-09 16:19:12]

Description was changed from

==========
Rework CallApi*Stubs.

- Eliminate stubs with a variable number of arguments.
  (That only worked due to their very limited use. These
   stubs' interface descriptors were basically lying
   about their number of args, which will fail when used
   generically.)
- Fix all CallApi*Stubs' interface descriptors to no
  longer lie about their arguments.
- Unify CallApi*Stub, for * in Function, Accessor,
  FunctionWithFixedArgs.
  (Since these are now all doing the same thing.)
- Rename the unified stub (and interface descriptors) to
  *ApiCallback*, since that's really what they're doing.
- Refuse inlining an API callback if its number of
  parameters exceeds the supported number of args.

BUG=

Committed: https://crrev.com/d238b953a474272c0e3ea22ef6a9b63fa9729340
Cr-Commit-Position: refs/heads/master@{#34614}
==========

to

==========
Rework CallApi*Stubs.

- Eliminate stubs with a variable number of arguments.
  (That only worked due to their very limited use. These
   stubs' interface descriptors were basically lying
   about their number of args, which will fail when used
   generically.)
- Fix all CallApi*Stubs' interface descriptors to no
  longer lie about their arguments.
- Unify CallApi*Stub, for * in Function, Accessor,
  FunctionWithFixedArgs.
  (Since these are now all doing the same thing.)
- Rename the unified stub (and interface descriptors) to
  *ApiCallback*, since that's really what they're doing.
- Refuse inlining an API callback if its number of
  parameters exceeds the supported number of args.

BUG=

Committed: https://crrev.com/d238b953a474272c0e3ea22ef6a9b63fa9729340
Cr-Commit-Position: refs/heads/master@{#34614}

Committed: https://crrev.com/5096492f1b0460b2644c7f79f9ccc5881fc65c50
Cr-Commit-Position: refs/heads/master@{#34627}
==========

[commit-bot: I haz the power, 2016-03-09 16:19:14]

Patchset 5 (id:??) landed as
https://crrev.com/5096492f1b0460b2644c7f79f9ccc5881fc65c50
Cr-Commit-Position: refs/heads/master@{#34627}