Optimize various forms of slice.call(arguments, ...)

Optimize various forms of slice.call(arguments, ...)

[Closed since this was split up in multiple patches]

BUG=v8:3037
LOG=n
R=verwaest@chromium.org
TEST=mjsunit/compiler/slice-call-arguments

[Toon Verwaest, 2014-06-12 08:46:06]

I think this is the wrong approach to tackle this problem. And (again) I think
this should be split up into multiple CLs.

It's fine to hack something like this together to measure the benefits, but
we'll never land such local hard/hand-coded optimizations. Once you see that
benefits are to be had of a certain optimization, think of a general framework
that fits within what we already have. And make sure that either there can be
future extensions to the framework, or that your extension is really a
leaf-extension (such as inlining kArraySlice). But even in that example, there
should be a framework for inlining builtins, rather than a local hack, and a
single such framework that's usable by everyone who tries to inline builtins.

As for this CL, what you want to do is first support .call in a similar way to
how .apply works. It just sets up the method, receiver and arguments and make a
call (which may be inlined). Then use the regular "TryInlineBuiltinMethodCall"
to inline kArraySlice.

Lastly, finding out whether we are calling .call on Array.prototype.slice might
need some guessing, but you surely shouldn't be using a runtime call to locally
disable assumptions. There should be a general framework behind it.

But before we dive into that, I actually doubt it will even be necessary. If
it's actually written as Array.prototype.slice.call, or even [].slice.call, we
can load both Array.prototype and [] as HConstants, from which we can probably
gather that .slice is that builtin, etc. That by itself might need some extra
work to avoid deopts due to missing feedback. But whenever we have static
guarantees we shouldn't be inserting soft-deopts anyway (if we are).

[p.antonov, 2014-06-12 09:38:53]

The reason for this soft deopt approach is that there is no way to know in the
common case if the receiver of .call is the array slice function because it's a
context lookup. For example CoffeeScript and jQuery save a reference to slice
instad of using `Array.prototype.slice.call` or `[].slice.call`.

From the detected argument patterns it's a pretty safe assumption that in
reasonable code it will be reference to slice, but if it's not, it shouldn't
just go into a deoptimization loop, should it?

Only doing this optimization when we know the receiver of .call is the array
slice function (e.g. in Array.prototype.slice.call and [].slice.call) could be
done very cleanly and within the existing framework yes, but it would have a
much smaller impact. In fact the feature request mentions CoffeeScript which
wouldn't benefit.

So we could:

1. Settle for a small impact but not create any new frameworks

2. Create a framework for risky but reasonable optimistic assumptions that
cannot be validated until the optimized code actually runs and then marking such
functions if these assumptions fail to avoid going into a deopt loop. The CL
currently implements this.

[Toon Verwaest, 2014-06-12 09:49:24]

So the right way to do this is to detect constant context slots. That way it
will have the big impact while having a clean implementation.

[p.antonov, 2014-06-12 09:52:11]

That would be best yes, but I was under the impression that with lazy parsing
it's not feasible. Like wouldn't you have to go and inspect all the functions
that share the same context?

[Toon Verwaest, 2014-06-12 10:02:50]

There are multiple ways of doing it, the most conservative of which would just
use dependencies and deoptimization when the slot is mutated.

We do the same thing for global variables already. They are assumed to be
constant, and their values are inlined as long as they stay constant. Upon
inlining you add a dependency to the global variable that will deoptimize the
code when someone mutates the global variable.

We still haven't decided what the best approach is for context slots, but at
least something like that should work.

If you want to continue working on this, I suggest you go ahead and do the other
work packages that I suggested, and presume that we'll have some form of
constant context slot tracking at some point.

[p.antonov, 2014-06-12 10:14:54]

Ok, sounds good :)

I assume it should be 2 patches, right?:

1. For recognizing .call in general to call tryinlinebuiltinmethodcall when the
receiver of .call is known

2. Adding kArraySlice case to tryinlinebuiltinmethodcall

[Toon Verwaest, 2014-06-12 11:27:43]

Yes exactly.