I forgot to add that this change is for ia32 only. ARM and x64 changes are only
made to make them compile.

http://codereview.chromium.org/2280007/diff/1/20
File src/arm/ic-arm.cc (right):

http://codereview.chromium.org/2280007/diff/1/20#newcode293
src/arm/ic-arm.cc:293: CallModeFlag call_mode) {
I forgot to tell everyone that this change is for ia32 only. Changes to
ARM and X64 only made to make them compile.

On 2010/05/27 11:19:36, antonm wrote:
why call_mode is not used here and in methods below.

http://codereview.chromium.org/2280007/diff/1/16
File src/codegen.cc (right):

http://codereview.chromium.org/2280007/diff/1/16#newcode258
src/codegen.cc:258: StubCache::ComputeCallInitialize(argc, in_loop,
FIXED_NAME), Code);
Yes, absolutely.
On 2010/05/27 11:04:52, antonm wrote:
NAMED_CALL?

http://codereview.chromium.org/2280007/diff/1/5
File src/globals.h (right):

http://codereview.chromium.org/2280007/diff/1/5#newcode462
src/globals.h:462: FIXED_NAME,
I am not happy with the names either, thanks for the suggestion. I
actually like NAMED_CALL/KEYED_CALL. Will change to that if no one comes
up with better idea.
On 2010/05/27 11:04:52, antonm wrote:
sorry for repetition: I'd prefer to see NAMED_CALL/MODE,
KEYED_CALL/MODE

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

http://codereview.chromium.org/2280007/diff/1/6#newcode5963
src/ia32/codegen-ia32.cc:5963: // Push the name of the function onto the
frame.
On 2010/05/27 11:04:52, antonm wrote:
Load the name?

Done.

http://codereview.chromium.org/2280007/diff/1/7
File src/ia32/full-codegen-ia32.cc (right):

http://codereview.chromium.org/2280007/diff/1/7#newcode1851
src/ia32/full-codegen-ia32.cc:1851: if (prop->is_synthetic()) {
Yes, I just moved the whole block of code under the condition without
really reading it. Simplified.
On 2010/05/27 11:19:36, antonm wrote:
that is weird: you've checked prop->is_synthetic above, don't you?

http://codereview.chromium.org/2280007/diff/1/7#newcode1860
src/ia32/full-codegen-ia32.cc:1860: // instruction after the call it is
treated specially
On 2010/05/27 11:04:52, antonm wrote:
the call <as> it is ?  I see you just moved it, but maybe you can
correct while
you're here.

Done.

http://codereview.chromium.org/2280007/diff/1/13
File src/ic.cc (right):

http://codereview.chromium.org/2280007/diff/1/13#newcode160
src/ic.cc:160: (kind == Code::CALL_IC && target->ic_call_mode() ==
VARIABLE_NAME)) {
It is redundant. Removed.
On 2010/05/27 11:04:52, antonm wrote:
why you added CallIIC case check here?

http://codereview.chromium.org/2280007/diff/1/13#newcode1374
src/ic.cc:1374: ic.KeyedLoadFunction(state, args.at<Object>(0),
args.at<Object>(1));
On 2010/05/27 11:04:52, antonm wrote:
nit: indent, I'd add two more spaces

Done.

http://codereview.chromium.org/2280007/diff/1/13#newcode1376
src/ic.cc:1376: // The first time the inline cache is updated may be the
first time the
Tried that. An extra call slows things down by 0.5-1.0%. Will look into
it again.
On 2010/05/27 11:04:52, antonm wrote:
I wonder if the code below should be refactored into a separate
function.

http://codereview.chromium.org/2280007/show

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

Reply via email to