Please cut this down to the minimal possible change. Do not attempt to
handle
three different variants of runtime calls in one change. Just split them
into
two at first and then add another category later if needed.
Also, there seems to be a renaming change in here as well in a test. Could
you
split that out as a separate change?
First batch of comments below. I'll have a look again when this has been
cleaned up to only contain a subset of this.
http://codereview.chromium.org/598072/diff/8001/8009
File src/ia32/ic-ia32.cc (right):
http://codereview.chromium.org/598072/diff/8001/8009#newcode628
src/ia32/ic-ia32.cc:628: IC_Utility(kKeyedLoadPropertyWithInterceptor)),
2, 1);
I think this is easier to read:
__ TailCallExternalReference(
ExternalReference(IC_Utility(kKeyedLoadPropertyWithInterceptor)),
2,
1);
http://codereview.chromium.org/598072/diff/8001/8009#newcode1280
src/ia32/ic-ia32.cc:1280: 2, 1);
Align the arguments with the other argument to
TailCallExternalReference:
__
TailCallExternalReference(ExternalReference(IC_Utility(kLoadIC_Miss)),
2,
1);
http://codereview.chromium.org/598072/diff/8001/8009#newcode1395
src/ia32/ic-ia32.cc:1395: __
TailCallExternalReference(ExternalReference(IC_Utility(
Identation as above.
http://codereview.chromium.org/598072/diff/8001/8009#newcode1450
src/ia32/ic-ia32.cc:1450: __
TailCallExternalReference(ExternalReference(IC_Utility(kStoreIC_Miss)),
Identation.
http://codereview.chromium.org/598072/diff/8001/8009#newcode1492
src/ia32/ic-ia32.cc:1492: __
TailCallExternalReference(ExternalReference(IC_Utility(
Indentation.
http://codereview.chromium.org/598072/diff/8001/8003
File src/ia32/macro-assembler-ia32.cc (right):
http://codereview.chromium.org/598072/diff/8001/8003#newcode1163
src/ia32/macro-assembler-ia32.cc:1163: void
MacroAssembler::DirectCallRuntime(Runtime::Function* f, bool tail) {
When a method in the macro-assembler handles both calls and jumps
(tail-calls), we use the convention that it is called InvokeXYZ and that
it takes an InvokeFlag parameter which is either CALL_FUNCTION or
JUMP_FUNCTION.
So, please call this one DirectInvokeRuntime or something like that and
replace the bool with an InvokeFlag parameter.
http://codereview.chromium.org/598072/diff/8001/8003#newcode1165
src/ia32/macro-assembler-ia32.cc:1165: // ag1
ag1 -> arg1
http://codereview.chromium.org/598072/diff/8001/8003#newcode1267
src/ia32/macro-assembler-ia32.cc:1267: int num_arguments,
Indentation.
http://codereview.chromium.org/598072/diff/8001/8004
File src/ia32/macro-assembler-ia32.h (right):
http://codereview.chromium.org/598072/diff/8001/8004#newcode377
src/ia32/macro-assembler-ia32.h:377: int num_arguments,
Align with the other arguments.
http://codereview.chromium.org/598072/diff/8001/8005
File src/runtime.cc (right):
http://codereview.chromium.org/598072/diff/8001/8005#newcode52
src/runtime.cc:52: #if V8_TARGET_ARCH_IA32
Get rid of these defines.
http://codereview.chromium.org/598072/diff/8001/8005#newcode59
src/runtime.cc:59: class NotFailsTag {};
Why do you need this?
http://codereview.chromium.org/598072/diff/8001/8005#newcode8273
src/runtime.cc:8273: // Utilities for building the runtime function list
I would like to get rid of all of this. Can we split up the C++ runtime
functions in a number of lists (macros) instead and get rid of all this?
http://codereview.chromium.org/598072/diff/8001/8006
File src/runtime.h (right):
http://codereview.chromium.org/598072/diff/8001/8006#newcode372
src/runtime.h:372: // and collect the garbage.
collect the garbage -> perform garbage collection.
http://codereview.chromium.org/598072/diff/8001/8006#newcode379
src/runtime.h:379: // The Callee always returns a valid object
Callee -> callee
Please end comment with period.
http://codereview.chromium.org/598072/diff/8001/8011
File src/x64/macro-assembler-x64.cc (right):
http://codereview.chromium.org/598072/diff/8001/8011#newcode384
src/x64/macro-assembler-x64.cc:384: == f->calling_convention) ? 0 :
num_arguments;
Can we indent this differently please?
http://codereview.chromium.org/598072
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
