On 2015/09/01 04:02:15, Benedikt Meurer wrote:
On 2015/09/01 03:55:02, caitp wrote:
> https://codereview.chromium.org/1272673003/diff/150001/src/parser.cc
> File src/parser.cc (right):
>
>
https://codereview.chromium.org/1272673003/diff/150001/src/parser.cc#newcode4415
> src/parser.cc:4415: // for (var $argument_index = $rest_index;
> On 2015/09/01 03:09:10, Benedikt Meurer wrote:
> > I guess there was some discussion about this desugaring; so do we
have any
> plans
> > how to make this work with inlining? esp. inlining in TurboFan?
>
> I don't think there are plans per se (but you might have seen there was
some
TF
> code previously for this, not sure how complete it was in terms of
inlining).
>
> There are at least some ideas about how the runtime call could be
changed
into
> something relatively quick. The spec'd algorithm uses
CreateDataProperty(O,
P,
> V) to add the element to the array. For rest parameters, we can make a
lot
of
> assumptions which lend themselves well to inlining:
>
> - the rest array's own "length" property is always writable, never
configurable
> - the rest array is always extensible
> - the rest array's own indexed properties are always writable and
configurable
> (they aren't present until the rest parameter algorithm sets them up)
>
> Based on this, yes, the current implementation probably isn't doing the
right
> thing, but it's pretty close.
>
> I guess there are also statistical guesses about the size of the array
to
> allocate, and whether or not to perform the loop at all, after a bit of
> profiling. But maybe compiler-folks are better for talking about that
stuff.
I'm more worried about the %_ArgumentsLength and %_Argument intrinsics.
Not
sure
if
and how we can support that with inlining, esp. %_Argument(i) with
non-constant
i will
probably need to be turned into some kind of switch during inlining. From
a
compiler
perspective (and without deep understanding of rest parameters, yet), I'd
think
that a
dedicated RestParameter AST node, which is then turned into a
JSCreateRestParameters
graph node is easiest to optimize wrt. inlining; as soon as the inliner
discovers a
JSCreateRestParameters (which should hang off the Start node), it will
replace
it with a
JSArray containing all the (then statically) known rest parameters.
Otherwise
the rest
parameters are lowered to a runtime call or a stub call, which is
responsible
to
allocate
the JSArray.
Thinking more about this, desugaring to a loop will probably generate
quite slow code in Crankshaft and fullcodegen as well, compared to
just a single runtime/stub call that collects all arguments into a JSArray.
If we'd have something like JSCreateArgumentsArray(start_index), backed
by a runtime call and stub, then we could use that for both desugaring
rest parameters and arguments.
I don't want to block you on this, but I have the feeling that we might
need to think harder about the arguments/rest parameter story.
https://codereview.chromium.org/1272673003/
--
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
---
You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
For more options, visit https://groups.google.com/d/optout.
