Addressed comments, PTAL again.
https://chromiumcodereview.appspot.com/23537067/diff/6001/src/code-stubs-hydrogen.cc
File src/code-stubs-hydrogen.cc (right):
https://chromiumcodereview.appspot.com/23537067/diff/6001/src/code-stubs-hydrogen.cc#newcode586
src/code-stubs-hydrogen.cc:586: }
On 2013/10/02 08:49:11, danno wrote:
This is a total hack. I am not sure how to do it better, but my
intuition is
that it more properly belongs in the the graph or compiler-specific
pieces that
calculate the size of the parameter area. Maybe the graph builder
needs a
virtual method to compute the parameter count that by default uses the
existing
logic and is overridden in this case to look at argc.
Done.
https://chromiumcodereview.appspot.com/23537067/diff/6001/src/code-stubs-hydrogen.cc#newcode598
src/code-stubs-hydrogen.cc:598: map_array, kind_index,
static_cast<HValue*>(NULL), FAST_ELEMENTS);
On 2013/10/02 08:49:11, danno wrote:
Please find a way to share this code in some way with
HGraphBuilder::JSArrayBuilder::EmitMapCode.
Done.
https://chromiumcodereview.appspot.com/23537067/diff/6001/src/code-stubs.h
File src/code-stubs.h (right):
https://chromiumcodereview.appspot.com/23537067/diff/6001/src/code-stubs.h#newcode1039
src/code-stubs.h:1039: class ArgcBits: public BitField<int, 5, 20> {};
On 2013/10/02 08:49:11, danno wrote:
Don't we have a constant somewhere with the number of max args and max
arg bits?
Please try to link them together somehow, either by using a shared
constant or
adding a STATIC_ASSERT() here that depends on existing constants.
Done.
https://chromiumcodereview.appspot.com/23537067/diff/6001/src/code-stubs.h#newcode2360
src/code-stubs.h:2360: class FPRegisters: public BitField<bool,
0, 1> {};
On 2013/10/02 08:49:11, danno wrote:
nit: crazy unnecessary spacing on the line above.
Done.
https://chromiumcodereview.appspot.com/23537067/diff/6001/src/deoptimizer.cc
File src/deoptimizer.cc (right):
https://chromiumcodereview.appspot.com/23537067/diff/6001/src/deoptimizer.cc#newcode1428
src/deoptimizer.cc:1428: PrintF(" translating %s =>
CallStubFailureTrampolineStub, height=%d\n",
On 2013/10/02 08:49:11, danno wrote:
You can use a single statement and just switch the stub kind string
with a %s
argument.
Done.
https://chromiumcodereview.appspot.com/23537067/diff/6001/src/deoptimizer.cc#newcode1580
src/deoptimizer.cc:1580:
CallStubFailureTrampolineStub().FindCodeInCache(&trampoline, isolate_);
On 2013/10/02 08:49:11, danno wrote:
Is this only going to every be used with CallStubs? It would be nice
if the
trampoline didn't have such a specific name, e.g.
StubFailureNormalContinuationTrampoline and
StubfailureTailCallContinuationTrampoline rather than CallStub
specific.
Done.
https://chromiumcodereview.appspot.com/23537067/diff/6001/src/hydrogen-instructions.h
File src/hydrogen-instructions.h (right):
https://chromiumcodereview.appspot.com/23537067/diff/6001/src/hydrogen-instructions.h#newcode2194
src/hydrogen-instructions.h:2194: bool tailcall)
On 2013/10/02 08:49:11, danno wrote:
Can you please turn this boolean into an enum so that it's clear at
the call
site what the meaning of the parameter is, e.g.
enum CallFunctionMode {
NORMAL_FUNCTION_CALL,
TAIL_FUNCTION_CALL
}
This would need to get propagated through the various signatures and
storage
sites, too.
Done.
https://chromiumcodereview.appspot.com/23537067/diff/6001/src/ia32/lithium-codegen-ia32.cc
File src/ia32/lithium-codegen-ia32.cc (right):
https://chromiumcodereview.appspot.com/23537067/diff/6001/src/ia32/lithium-codegen-ia32.cc#newcode4375
src/ia32/lithium-codegen-ia32.cc:4375: if
(instr->hydrogen()->tailcall()) {
On 2013/10/02 08:49:11, danno wrote:
this should probably be called IsTailCall()
Done.
https://chromiumcodereview.appspot.com/23537067/diff/6001/src/ic.cc
File src/ic.cc (right):
https://chromiumcodereview.appspot.com/23537067/diff/6001/src/ic.cc#newcode2419
src/ic.cc:2419: Object* key = args[0];
On 2013/10/02 08:49:11, danno wrote:
Oh, this is awful. Please take a look at ArrayConstructor, which
through the
CodeStubFailureTrampoline has access to the caller's arguments in
args[0].
Please extend the current stub failure mechanism to set up the failure
trampoline frame correctly so that you can just do:
Arguments callerArgs* = reinterpret_cast<Arguments*>(args[0]);
Done.
https://chromiumcodereview.appspot.com/23537067/
--
--
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/groups/opt_out.
