https://codereview.chromium.org/335683002/diff/1/src/hydrogen.cc
File src/hydrogen.cc (right):
https://codereview.chromium.org/335683002/diff/1/src/hydrogen.cc#newcode7978
src/hydrogen.cc:7978: bool
HOptimizedGraphBuilder::TryInlineBuiltinMethodCall(
On 2014/06/12 21:41:13, Toon Verwaest wrote:
Does it make sense to have both methods? I'm pretty sure that by
having both,
people will start calling the other version, which may not be correct?
Done.
https://codereview.chromium.org/335683002/diff/1/src/hydrogen.cc#newcode7984
src/hydrogen.cc:7984: ) {
On 2014/06/12 21:41:13, Toon Verwaest wrote:
Nit: put ) { back on the previous line
Done.
https://codereview.chromium.org/335683002/diff/1/src/hydrogen.cc#newcode8005
src/hydrogen.cc:8005: // expr could be .call or .apply so it's not safe
to rely on its
On 2014/06/12 21:41:13, Toon Verwaest wrote:
I don't think we need this comment...
Done.
https://codereview.chromium.org/335683002/diff/1/src/hydrogen.cc#newcode8570
src/hydrogen.cc:8570: if (TryInlineApply(known_function, expr,
args_count_no_receiver)) {
On 2014/06/12 21:41:13, Toon Verwaest wrote:
Seems like TryIndirectCall splits to apply and call handling, which
join again
in HandleIndirectCall. It's a bit weird that it here again has special
handling
for Apply. Is it really specific to Apply, or should it just be named
TryInlineIndirectCall?
Or perhaps we can even merge it with TryInlineCall since they don't
behave that
different anymore.
Done.
https://codereview.chromium.org/335683002/diff/1/src/hydrogen.cc#newcode8575
src/hydrogen.cc:8575: HValue* receiver =
environment()->ExpressionStackAt(args_count_no_receiver);
On 2014/06/12 21:41:13, Toon Verwaest wrote:
Can you move this code into TryInlineBuiltinMethodCall?
Done.
https://codereview.chromium.org/335683002/diff/1/src/hydrogen.cc#newcode8581
src/hydrogen.cc:8581: if (TryInlineBuiltinMethodCall(expr, receiver,
map, known_function,
On 2014/06/12 21:41:13, Toon Verwaest wrote:
This was already done in BuildFunctionCallCall. Can we avoid this
duplication?
Either pass in a flag for the extra behavior, or move this out to the
Apply
variant. Well, I guess we first have to think about the extra behavior
tout
court... see the comment down below.
Done.
https://codereview.chromium.org/335683002/diff/1/src/hydrogen.cc#newcode8603
src/hydrogen.cc:8603: !expr->target()->shared()->HasBuiltinFunctionId())
{
On 2014/06/12 21:41:13, Toon Verwaest wrote:
What about creating both BuildFunctionCall and BuildFunctionApply, and
using a
switch to jump to those? I think you did that in your previous CL...
That makes
it a bit more symmetrical.
Done.
https://codereview.chromium.org/335683002/diff/1/src/hydrogen.cc#newcode8686
src/hydrogen.cc:8686: receiver = is_builtin_call ? receiver :
BuildWrapReceiver(receiver, function);
On 2014/06/12 21:41:14, Toon Verwaest wrote:
You can't just skip BuildWrapReceiver for builtins. BuildWrapReceiver
has the
required logic to figure out whether it can be skipped or not (skipped
only when
it's in strict-mode). Slice is in strict-mode, which is what you care
about
most, so that should work out I presume?
Done.
https://codereview.chromium.org/335683002/diff/1/src/hydrogen.cc#newcode8710
src/hydrogen.cc:8710: // be allowed after all.
On 2014/06/12 21:41:13, Toon Verwaest wrote:
Iek! So what if I do the following?
a = [];
a.push(arguments);
return a;
Done.
https://codereview.chromium.org/335683002/
--
--
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.
