[runtime] Move binary operator fallbacks into the runtime.

[runtime] Move binary operator fallbacks into the runtime.

Replace the ADD, SUB, etc. builtins with proper runtime implementations,
and expose them as runtime calls that can be used by the code stubs and
the interpreter (for now).

Also remove all the support runtime functions for ADD, SUB and friends,
namely %NumberAdd, %NumberSub, and so on.

R=mstarzinger@chromium.org
CQ_INCLUDE_TRYBOTS=tryserver.v8:v8_linux_layout_dbg,v8_linux_nosnap_dbg

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

[Benedikt Meurer, 2015-09-10 09:47:27]

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

[Benedikt Meurer, 2015-09-10 09:47:28]

Michi: Please review the compiler and runtime changes.
Ross: Please check interpreter changes.

[Benedikt Meurer, 2015-09-10 09:49:34]

https://codereview.chromium.org/1333843002/diff/1/test/webkit/function-apply-...
File test/webkit/function-apply-aliased.js (left):

https://codereview.chromium.org/1333843002/diff/1/test/webkit/function-apply-...
test/webkit/function-apply-aliased.js:71: function stackOverflowTest() {
Note: This test was testing specific stack overflow behavior, which is now
slightly different, so I discussed with machenbach@ and we decided to drop the
fragile part of the test.

[Benedikt Meurer, 2015-09-10 09:53:21]

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

[commit-bot: I haz the power, 2015-09-10 09:53:31]

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

[rmcilroy, 2015-09-10 10:28:25]

A high level question about frame states.

https://codereview.chromium.org/1333843002/diff/1/src/compiler/interpreter-as...
File src/compiler/interpreter-assembler.h (right):

https://codereview.chromium.org/1333843002/diff/1/src/compiler/interpreter-as...
src/compiler/interpreter-assembler.h:119: // Returns an empty frame (used as a
placeholder until we can actuall deopt).
We will never be deopting the interpreter bytecode handlers, so this isn't a
placeholder. We should really only be calling runtime functions which can't
cause deopts - what is the situation for the new runtime functions, why do they
need FrameStates? Could we special case them in Linkage::FrameStateInputCount so
that they don't require frame states?

https://codereview.chromium.org/1333843002/diff/1/test/unittests/compiler/int...
File test/unittests/compiler/interpreter-assembler-unittest.cc (right):

https://codereview.chromium.org/1333843002/diff/1/test/unittests/compiler/int...
test/unittests/compiler/interpreter-assembler-unittest.cc:330: 
Please add a CallRuntime test here

[Benedikt Meurer, 2015-09-10 10:31:12]

https://codereview.chromium.org/1333843002/diff/1/src/compiler/interpreter-as...
File src/compiler/interpreter-assembler.h (right):

https://codereview.chromium.org/1333843002/diff/1/src/compiler/interpreter-as...
src/compiler/interpreter-assembler.h:119: // Returns an empty frame (used as a
placeholder until we can actuall deopt).
Hm, you're right, the frame state can be empty here, because it's only required
to fullfil the API needs. So what you're suggesting is basically a special
GetRuntimeCallDescriptorForInterpreter, which avoids the frame state input?

[rmcilroy, 2015-09-10 10:43:22]

https://codereview.chromium.org/1333843002/diff/1/src/compiler/interpreter-as...
File src/compiler/interpreter-assembler.h (right):

https://codereview.chromium.org/1333843002/diff/1/src/compiler/interpreter-as...
src/compiler/interpreter-assembler.h:119: // Returns an empty frame (used as a
placeholder until we can actuall deopt).
On 2015/09/10 10:31:12, Benedikt Meurer wrote:
> Hm, you're right, the frame state can be empty here, because it's only
required
> to fullfil the API needs. So what you're suggesting is basically a special
> GetRuntimeCallDescriptorForInterpreter, which avoids the frame state input?

If it's safe to do this, then yeah I would be fine with it (it could also just
be an option passed to GetRuntimeCallDescriptor since that function already
potentially returns a CallDescriptor::kNoFlags if Linkage::FrameStateInputCount
says the function doesn't need a frame state.

[Benedikt Meurer, 2015-09-10 10:55:49]

Ok, we don't need the frame state now :-)

https://codereview.chromium.org/1333843002/diff/1/test/unittests/compiler/int...
File test/unittests/compiler/interpreter-assembler-unittest.cc (right):

https://codereview.chromium.org/1333843002/diff/1/test/unittests/compiler/int...
test/unittests/compiler/interpreter-assembler-unittest.cc:330: 
On 2015/09/10 10:28:25, rmcilroy wrote:
> Please add a CallRuntime test here 

Done.

[rmcilroy, 2015-09-10 11:16:34]

Interpreter changes lgtm. One suggestion in linkage.h/cc. I didn't look at the
other changes.

https://codereview.chromium.org/1333843002/diff/20001/src/compiler/linkage.h
File src/compiler/linkage.h (right):

https://codereview.chromium.org/1333843002/diff/20001/src/compiler/linkage.h#...
src/compiler/linkage.h:276: Operator::Properties properties, bool
needs_frame_state = true);
nit - could you make this an enum instead of a bool for code clarity please.

[Benedikt Meurer, 2015-09-10 11:26:51]

https://codereview.chromium.org/1333843002/diff/20001/src/compiler/linkage.h
File src/compiler/linkage.h (right):

https://codereview.chromium.org/1333843002/diff/20001/src/compiler/linkage.h#...
src/compiler/linkage.h:276: Operator::Properties properties, bool
needs_frame_state = true);
Mhm, I think that is a bit overkill for the one case where you actually see the
parameter.

[Michael Starzinger, 2015-09-10 11:43:24]

LGTM.

https://codereview.chromium.org/1333843002/diff/20001/src/hydrogen.cc
File src/hydrogen.cc (right):

https://codereview.chromium.org/1333843002/diff/20001/src/hydrogen.cc#newcode...
src/hydrogen.cc:11072: UNREACHABLE();
Sneaky fall-through to convince compiler that function_id is initialized is
being sneaky. :)

https://codereview.chromium.org/1333843002/diff/20001/src/runtime/runtime.h
File src/runtime/runtime.h (right):

https://codereview.chromium.org/1333843002/diff/20001/src/runtime/runtime.h#n...
src/runtime/runtime.h:526: 
nit: Two empty newlines.

[Benedikt Meurer, 2015-09-10 11:44:31]

The CQ bit was checked by bmeurer@chromium.org

[commit-bot: I haz the power, 2015-09-10 11:44:45]

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

[commit-bot: I haz the power, 2015-09-10 13:04:21]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2015-09-10 13:04:39]

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