PTAL
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)); On 2012/04/11 11:49:18, Kevin Millikin wrote:
I'd be more comfortable if we set this flag on the Hydrogen
instruction instead. Done. 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> { On 2012/04/11 11:49:18, Kevin Millikin wrote:
I think we normally use the terminology "Drop" for something that can
pop more
than one and "Pop" for popping exactly one.
Done. 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)); Off by one is there to avoid changing access code. On 2012/04/11 11:49:18, Kevin Millikin wrote:
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()) { On 2012/04/11 11:49:18, Kevin Millikin wrote:
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.
Done. 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_; On 2012/04/11 11:49:18, Kevin Millikin wrote:
Why not HEnterInlined* and HArgumentsElements* below?
Done. http://codereview.chromium.org/10033028/ -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
