I have issues.
http://codereview.chromium.org/10033028/diff/1/src/arm/lithium-arm.cc File src/arm/lithium-arm.cc (right): http://codereview.chromium.org/10033028/diff/1/src/arm/lithium-arm.cc#newcode1081 src/arm/lithium-arm.cc:1081: current_block_->last_environment()->outer() != NULL)); I'd be more comfortable if we set this flag on the Hydrogen instruction instead. http://codereview.chromium.org/10033028/diff/1/src/arm/lithium-arm.h File src/arm/lithium-arm.h (right): http://codereview.chromium.org/10033028/diff/1/src/arm/lithium-arm.h#newcode1388 src/arm/lithium-arm.h:1388: class LPop: public LTemplateInstruction<0, 0, 0> { I think we normally use the terminology "Drop" for something that can pop more than one and "Pop" for popping exactly one. http://codereview.chromium.org/10033028/diff/1/src/arm/lithium-codegen-arm.cc File src/arm/lithium-codegen-arm.cc (right): http://codereview.chromium.org/10033028/diff/1/src/arm/lithium-codegen-arm.cc#newcode2768 src/arm/lithium-codegen-arm.cc:2768: __ add(result, sp, Operand(-2 * kPointerSize)); add? sub? I think it's weird to have this off-by-two in both the inlined and non-inlined cases. For the inlined case there is a sub here and then an add at every element access to compensate. For the non-inlined case there is an add at every element access instead of just having one here. I guess the reason is that ArgumentsLength would then have to change, but the way we do that seems like it could be better, too. (Setting a register to either fp or not fp, then comparing it to fp to see if we set it to fp or not fp, then issuing a duplicate memory load of caller fp if we have an adaptor....) http://codereview.chromium.org/10033028/diff/1/src/hydrogen.cc File src/hydrogen.cc (right): http://codereview.chromium.org/10033028/diff/1/src/hydrogen.cc#newcode5005 src/hydrogen.cc:5005: if (!function_state()->arguments_pushed()) { I'd move this after all possible "return false", so that the graph is not mutated in the case we return false. That might require duplicating the code. If so, factor it out into a separate function. http://codereview.chromium.org/10033028/diff/1/src/hydrogen.h File src/hydrogen.h (right): http://codereview.chromium.org/10033028/diff/1/src/hydrogen.h#newcode757 src/hydrogen.h:757: HInstruction* entry_; Why not HEnterInlined* and HArgumentsElements* below? http://codereview.chromium.org/10033028/ -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
