LGTM with some comments.
http://codereview.chromium.org/115744/diff/1/19 File src/arm/codegen-arm.cc (right): http://codereview.chromium.org/115744/diff/1/19#newcode1060 Line 1060: CallFunctionStub(int argc, InlineCacheInLoop in_loop) I like a name that indicates flaggishness. InLoopFlag? http://codereview.chromium.org/115744/diff/1/18 File src/arm/ic-arm.cc (right): http://codereview.chromium.org/115744/diff/1/18#newcode425 Line 425: Code::Flags flags = Code::ComputeFlags(Code::LOAD_IC, NOT_IN_LOOP, MONOMORPHIC); Long line? http://codereview.chromium.org/115744/diff/1/18#newcode758 Line 758: Code::Flags flags = Code::ComputeFlags(Code::STORE_IC, NOT_IN_LOOP, MONOMORPHIC); Long line? http://codereview.chromium.org/115744/diff/1/17 File src/arm/stub-cache-arm.cc (right): http://codereview.chromium.org/115744/diff/1/17#newcode500 Line 500: Code::Flags flags) { Since you've generalized the call site to take flags, you might want to assert here that the passed flags has type FIELD. http://codereview.chromium.org/115744/diff/1/17#newcode552 Line 552: Code::Flags flags) { Same as above with type CONSTANT_FUNCTION. http://codereview.chromium.org/115744/diff/1/13 File src/codegen.cc (right): http://codereview.chromium.org/115744/diff/1/13#newcode330 Line 330: if (in_loop) { My opinion is mixed about making these two value, on/off flags into enums (for readability at the call site). But since you didn't make it a bool, please write: if (in_loop == IN_LOOP) ... http://codereview.chromium.org/115744/diff/1/5 File src/ia32/stub-cache-ia32.cc (right): http://codereview.chromium.org/115744/diff/1/5#newcode474 Line 474: Code::Flags flags) { Same comment about ASSERT as in the ARM file. http://codereview.chromium.org/115744/diff/1/5#newcode523 Line 523: Code::Flags flags) { And here. http://codereview.chromium.org/115744/diff/1/8 File src/ic.cc (right): http://codereview.chromium.org/115744/diff/1/8#newcode451 Line 451: state == PREMONOMORPHIC || state == MONOMORPHIC || One disjunct per line? http://codereview.chromium.org/115744/diff/1/8#newcode1111 Line 1111: if (in_loop) { if (in_loop == IN_LOOP) .... http://codereview.chromium.org/115744/diff/1/16 File src/objects-inl.h (right): http://codereview.chromium.org/115744/diff/1/16#newcode1957 Line 1957: if (in_loop) bits |= kFlagsICInLoopMask; if (in_loop == IN_LOOP) ... http://codereview.chromium.org/115744 --~--~---------~--~----~------------~-------~--~----~ v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev -~----------~----~----~----~------~----~------~--~---
