[Interpreter] Add support for JS runtime calls.

[Interpreter] Add support for JS runtime calls.

Adds support for calling JS runtime functions. Also changes the bytecode
array builder to allow calling functions with an invalid argument
register if the call takes no arguments.

Adds the bytecode CallJSRuntime.

BUG=v8:4280
LOG=N

Committed: https://crrev.com/41f3e782d968469f77095fd7464e7254944cc8d5
Cr-Commit-Position: refs/heads/master@{#31774}

[rmcilroy, 2015-10-30 17:15:47]

rmcilroy@chromium.org changed reviewers:
+ bmeurer@chromium.org, oth@chromium.org

[rmcilroy, 2015-10-30 17:15:48]

Benedikt, could you please take a look at compiler/
Orion, could you please take a look at interpreter/

Thanks.

[rmcilroy, 2015-10-30 17:16:53]

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

[commit-bot: I haz the power, 2015-10-30 17:17:03]

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

[commit-bot: I haz the power, 2015-10-30 17:29:08]

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

[commit-bot: I haz the power, 2015-10-30 17:29:09]

Dry run: Try jobs failed on following builders:
  v8_mac_rel on tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_mac_rel/builds/11365)

[rmcilroy, 2015-10-30 18:03:30]

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

[rmcilroy, 2015-10-30 18:03:37]

Patchset #1 (id:1) has been deleted

[commit-bot: I haz the power, 2015-10-30 18:03:44]

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

[commit-bot: I haz the power, 2015-10-30 18:27:21]

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

[commit-bot: I haz the power, 2015-10-30 18:27:22]

Dry run: This issue passed the CQ dry run.

[Benedikt Meurer, 2015-11-02 04:42:59]

bmeurer@chromium.org changed required reviewers:
+ oth@chromium.org

[Benedikt Meurer, 2015-11-02 04:42:59]

LGTM for compiler.

[oth, 2015-11-02 10:49:58]

Comments below. I can live with this change, but if the code could do something
cleaner with the register check that'd be great. Pre-caffeine it looks like
there's really one place that needs this. Strawmen below.

https://codereview.chromium.org/1410003003/diff/40001/src/interpreter/bytecod...
File src/interpreter/bytecode-generator.cc (left):

https://codereview.chromium.org/1410003003/diff/40001/src/interpreter/bytecod...
src/interpreter/bytecode-generator.cc:1573: first_arg =
execution_result()->NewRegister();
This may be the only spot that requires an unchecked register (?) as CallRuntime
needs a lead register and only has a single register operand.

We have a valid known register that doesn't entail any extra state in the other
cases (VisitCallNew has the contructor, VisitCallRuntime for JS Runtime call has
the receiver.

One alternative approach would be to have a method that gets the first valid
register (local, parameter, or a temporary: GetArbitraryRegisterOperand(). This
method would only be used if args->length() == 0 and most times would not entail
an additional temporary. The register would always appear valid to the bytecode
array builder.

Or we could have a MayReg8 operand which could be an invalid register. As you
mentioned this would remove the ability to combine the length == 0 check with
the invalid register check.

[rmcilroy, 2015-11-03 17:10:47]

https://codereview.chromium.org/1410003003/diff/40001/src/interpreter/bytecod...
File src/interpreter/bytecode-generator.cc (left):

https://codereview.chromium.org/1410003003/diff/40001/src/interpreter/bytecod...
src/interpreter/bytecode-generator.cc:1573: first_arg =
execution_result()->NewRegister();
On 2015/11/02 10:49:58, oth wrote:
> This may be the only spot that requires an unchecked register (?) as
CallRuntime
> needs a lead register and only has a single register operand.
> 
> We have a valid known register that doesn't entail any extra state in the
other
> cases (VisitCallNew has the contructor, VisitCallRuntime for JS Runtime call
has
> the receiver.
> 
> One alternative approach would be to have a method that gets the first valid
> register (local, parameter, or a temporary: GetArbitraryRegisterOperand().
This
> method would only be used if args->length() == 0 and most times would not
entail
> an additional temporary. The register would always appear valid to the
bytecode
> array builder.
> 
> Or we could have a MayReg8 operand which could be an invalid register. As you
> mentioned this would remove the ability to combine the length == 0 check with
> the invalid register check.
> 

There are two spots which need this - CallRuntime (here) and CallNew (which was
previously using the constructor register twice in these circumstances).

I've reworked it to remove the RegisterValidityCheck enum and instead have a
kMaybeReg8 which always accepts '0' as a valid operand, and do the actual check
that this only happens for arg_count == 0 in
BytecodeArrayBuilder::CallRuntime/CallNew. 

PTAL, thanks.

[oth, 2015-11-04 08:26:12]

lgtm

[rmcilroy, 2015-11-04 08:52:32]

The CQ bit was checked by rmcilroy@chromium.org

[rmcilroy, 2015-11-04 08:52:33]

The patchset sent to the CQ was uploaded after l-g-t-m from bmeurer@chromium.org
Link to the patchset: https://codereview.chromium.org/1410003003/#ps80001
(title: "Rebased")

[commit-bot: I haz the power, 2015-11-04 08:52:45]

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

[commit-bot: I haz the power, 2015-11-04 09:21:30]

Committed patchset #4 (id:80001)

[commit-bot: I haz the power, 2015-11-04 09:22:02]

Patchset 4 (id:??) landed as
https://crrev.com/41f3e782d968469f77095fd7464e7254944cc8d5
Cr-Commit-Position: refs/heads/master@{#31774}