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);
On 2010/02/19 10:49:57, Mads Ager wrote:
I think this is easier to read:
__ TailCallExternalReference(
ExternalReference(IC_Utility(kKeyedLoadPropertyWithInterceptor)),
2,
1);
I introduced a var to make this line shorter (like in some other places
of calling TailCallExternalReference).
http://codereview.chromium.org/598072/diff/8001/8009#newcode1280
src/ia32/ic-ia32.cc:1280: 2, 1);
On 2010/02/19 10:49:57, Mads Ager wrote:
Align the arguments with the other argument to
TailCallExternalReference:
__
TailCallExternalReference(ExternalReference(IC_Utility(kLoadIC_Miss)),
2,
1);
As above
http://codereview.chromium.org/598072/diff/8001/8009#newcode1395
src/ia32/ic-ia32.cc:1395: __
TailCallExternalReference(ExternalReference(IC_Utility(
On 2010/02/19 10:49:57, Mads Ager wrote:
Identation as above.
Done.
http://codereview.chromium.org/598072/diff/8001/8009#newcode1450
src/ia32/ic-ia32.cc:1450: __
TailCallExternalReference(ExternalReference(IC_Utility(kStoreIC_Miss)),
On 2010/02/19 10:49:57, Mads Ager wrote:
Identation.
Done.
http://codereview.chromium.org/598072/diff/8001/8009#newcode1492
src/ia32/ic-ia32.cc:1492: __
TailCallExternalReference(ExternalReference(IC_Utility(
On 2010/02/19 10:49:57, Mads Ager wrote:
Indentation.
Done.
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) {
On 2010/02/19 10:49:57, Mads Ager wrote:
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.
Done.
http://codereview.chromium.org/598072/diff/8001/8003#newcode1165
src/ia32/macro-assembler-ia32.cc:1165: // ag1
On 2010/02/19 10:49:57, Mads Ager wrote:
ag1 -> arg1
Done.
http://codereview.chromium.org/598072/diff/8001/8003#newcode1168
src/ia32/macro-assembler-ia32.cc:1168: // ret addr (if tail call)
On 2010/02/19 12:13:29, Søren Gjesse wrote:
I suggest encapsulating this in PrepareCallCFunction/CallCFunction in
the macro
assembler as on x64 and use these in the RegExp macro assembler. That
could be
done as a separate commit before this.
This code behaves differenly from x64's CallCFunction. However I'm
planning further optimization for ia32 what apparently will allow to
extract common code for CallCFunction.
http://codereview.chromium.org/598072/diff/8001/8003#newcode1211
src/ia32/macro-assembler-ia32.cc:1211:
call(FUNCTION_ADDR(ExternalReference(f).address()),
RelocInfo::RUNTIME_ENTRY);
On 2010/02/19 12:13:29, Søren Gjesse wrote:
Please add a comment saying that we are only performing a semi tail
call as we
need to copy the arguments for alignment. On Windows 32-bit we could
consider
cheating as the 8 byte alignment set in platform-win is not strictly
required.
We could measure this after this is done.
Done.
http://codereview.chromium.org/598072/diff/8001/8003#newcode1267
src/ia32/macro-assembler-ia32.cc:1267: int num_arguments,
On 2010/02/19 10:49:57, Mads Ager wrote:
Indentation.
Done.
http://codereview.chromium.org/598072/diff/8001/8004
File src/ia32/macro-assembler-ia32.h (right):
http://codereview.chromium.org/598072/diff/8001/8004#newcode369
src/ia32/macro-assembler-ia32.h:369: // Tail call of a runtime routine
(jump).
On 2010/02/19 12:13:29, Søren Gjesse wrote:
This comment needs to be changed as well.
Done.
http://codereview.chromium.org/598072/diff/8001/8004#newcode377
src/ia32/macro-assembler-ia32.h:377: int num_arguments,
On 2010/02/19 10:49:57, Mads Ager wrote:
Align with the other arguments.
Done.
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
On 2010/02/19 10:49:57, Mads Ager wrote:
Get rid of these defines.
Changed to:
#if defined(V8_TARGET_ARCH_X64) || defined(V8_TARGET_ARCH_IA32)
#define DIRECT_CALL_SUPPORTED
#endif
Did you mean that?
http://codereview.chromium.org/598072/diff/8001/8005#newcode59
src/runtime.cc:59: class NotFailsTag {};
On 2010/02/19 10:49:57, Mads Ager wrote:
Why do you need this?
I supposed to use it to differentiate function what never fails from
that could fail and need to be reties. Since code for function with
retry has removed it's not needed anymore. Deleted.
http://codereview.chromium.org/598072/diff/8001/8005#newcode8273
src/runtime.cc:8273: // Utilities for building the runtime function list
On 2010/02/19 10:49:57, Mads Ager wrote:
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?
It make easy to switch calling convention for a specific function. I'd
like to keep it for now. But since calling convention support will be
implemented on all architectures it could be done.
Alternatively we could use it for derivation all the function traits
(number of arguments and number of return values) from their signatures
simply making the Argument class be a template parametrized by number of
arguments (or even their types).
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.
On 2010/02/19 10:49:57, Mads Ager wrote:
collect the garbage -> perform garbage collection.
Done.
http://codereview.chromium.org/598072/diff/8001/8006#newcode379
src/runtime.h:379: // The Callee always returns a valid object
On 2010/02/19 10:49:57, Mads Ager wrote:
Callee -> callee
Please end comment with period.
Done.
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;
On 2010/02/19 10:49:57, Mads Ager wrote:
Can we indent this differently please?
Done.
http://codereview.chromium.org/598072/diff/8001/8011#newcode405
src/x64/macro-assembler-x64.cc:405: // A expected to be put into the
registers in order of Register
On 2010/02/19 12:13:29, Søren Gjesse wrote:
This comment seems a bit broken.
Done.
http://codereview.chromium.org/598072/diff/8001/8013
File src/x64/macro-assembler-x64.h (right):
http://codereview.chromium.org/598072/diff/8001/8013#newcode44
src/x64/macro-assembler-x64.h:44: static const RegList kCCallerSaved =
On 2010/02/19 12:13:29, Søren Gjesse wrote:
Can't we use Register.bit here (e.g. rax.bit())?
Done.
http://codereview.chromium.org/598072/diff/8001/8012
File src/x64/virtual-frame-x64.cc (right):
http://codereview.chromium.org/598072/diff/8001/8012#newcode982
src/x64/virtual-frame-x64.cc:982: Forget(arg_count);
On 2010/02/19 12:13:29, Søren Gjesse wrote:
On ia32 this threw and exception?
Not, just returns undefined value:
void MacroAssembler::IllegalOperation(int num_arguments) {
// ...
mov(eax, Immediate(Factory::undefined_value()));
}
http://codereview.chromium.org/598072/diff/8001/8012#newcode985
src/x64/virtual-frame-x64.cc:985:
On 2010/02/19 12:13:29, Søren Gjesse wrote:
This is more or less a duplication of what is in macro assembler
CallRuntime.
Why is that?
If you talk about checking parameters number it is duplication. I
wouldn't like to complicate the code supporting number of parameters
not- fitting in registers (any existing now candidate to be called
directly has not more than 3 parameters). Limiting number of parameters
by 4 would simplify code on ia32 (partially), x64 and arm.
May be we should just put ASSERT here?
I think you should start by just spilling all registers. And then
optimizing for calee saved registers later.
Actually I did it for ia32 (I home to come up with passing args in
registers, at least first 2). But in case of x64 this optimization looks
straightforward. What use of spilling all registers here?
http://codereview.chromium.org/598072
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
