http://codereview.chromium.org/5964005/diff/1/src/hydrogen-instructions.h
File src/hydrogen-instructions.h (right):

http://codereview.chromium.org/5964005/diff/1/src/hydrogen-instructions.h#newcode1219
src/hydrogen-instructions.h:1219: HOperandVector<1> operands_;
There are now 3 types of calls that take an extra operand: CallKeyed,
CallKnownGlobal and CallNew.

Maybe it is better to have a class HCallUnary that contains the common
functions for the operand. It would save some duplication.

http://codereview.chromium.org/5964005/diff/1/src/hydrogen.cc
File src/hydrogen.cc (right):

http://codereview.chromium.org/5964005/diff/1/src/hydrogen.cc#newcode113
src/hydrogen.cc:113: HInstruction* HBasicBlock::GetLastInstruction() {
I'd strongly favor removing this function together with the cached_
instruction in a separate change. I can't see a real benefit. start()
and end() should be enough, I think.

If a block is finished, we get get end_->previous(). As long as a block
is unfinished, we can update end_ along the way when adding
instructions.

http://codereview.chromium.org/5964005/diff/1/src/hydrogen.cc#newcode3238
src/hydrogen.cc:3238: default_graph->FinishExit(new HDeoptimize());
Leave out () after HDeoptimize.

http://codereview.chromium.org/5964005/diff/1/src/hydrogen.cc#newcode3550
src/hydrogen.cc:3550: default_graph->FinishExit(new HDeoptimize());
Leave out () after HDeoptimize.

http://codereview.chromium.org/5964005/diff/1/src/hydrogen.cc#newcode3896
src/hydrogen.cc:3896: // inlined variants (if any).  Otherwise use the
default.
Remove a space after .

http://codereview.chromium.org/5964005/diff/1/src/hydrogen.cc#newcode4056
src/hydrogen.cc:4056: HSubgraph* body = CreateInlinedSubgraph(env,
target, function, global_receiver);
Long line.

http://codereview.chromium.org/5964005/diff/1/src/hydrogen.cc#newcode4198
src/hydrogen.cc:4198: HValue* push_left =
environment()->ExpressionStackAt(1);
Can you write the following here instead?

ReplaceAndDeleteArguments(environment(), 3);
HValue* right = Pop();
HValue* left = Pop();
Pop();  // Pop receiver.

http://codereview.chromium.org/5964005/diff/1/src/hydrogen.cc#newcode4334
src/hydrogen.cc:4334: AddInstruction(push_receiver);
PushAndAdd(push_receiver);

http://codereview.chromium.org/5964005/diff/1/src/hydrogen.cc#newcode4478
src/hydrogen.cc:4478: Push(push_constructor);
could be shortened to:

PushAndAdd(new HPushArgument(constructor));

http://codereview.chromium.org/5964005/diff/1/src/hydrogen.cc#newcode5173
src/hydrogen.cc:5173: VisitArgumentList(call->arguments());
I'd replace all VisitArgumentList followed by a check for bailout with a
macro like VISIT_ARGUMENTS that includes the if-statement below.

http://codereview.chromium.org/5964005/diff/1/src/hydrogen.cc#newcode5224
src/hydrogen.cc:5224: ast_context()->ReturnInstruction(result,
call->id());
There seems a lot of duplicated code for all runtime-functions that call
a stub - and there will be more added. Maybe add a helper like
BuildStubCall?

http://codereview.chromium.org/5964005/diff/1/src/ia32/lithium-codegen-ia32.cc
File src/ia32/lithium-codegen-ia32.cc (right):

http://codereview.chromium.org/5964005/diff/1/src/ia32/lithium-codegen-ia32.cc#newcode2401
src/ia32/lithium-codegen-ia32.cc:2401: __ mov(Operand(esp, arity *
kPointerSize), eax);
It would be nice to avoid patching in the global receiver.

http://codereview.chromium.org/5964005/

--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev

Reply via email to