Hairy. LGTM.
https://chromiumcodereview.appspot.com/9265004/diff/4001/src/arm/deoptimizer-arm.cc File src/arm/deoptimizer-arm.cc (right): https://chromiumcodereview.appspot.com/9265004/diff/4001/src/arm/deoptimizer-arm.cc#newcode373 src/arm/deoptimizer-arm.cc:373: // The top address for the bottommost output frame can be computed from The first half of this comment can be dropped. Suggest: "The top address of the frame is computed from the previous frame's top and this frame's size." https://chromiumcodereview.appspot.com/9265004/diff/4001/src/arm/deoptimizer-arm.cc#newcode391 src/arm/deoptimizer-arm.cc:391: // Compute caller's PC Read caller's PC from the previous frame. https://chromiumcodereview.appspot.com/9265004/diff/4001/src/arm/deoptimizer-arm.cc#newcode401 src/arm/deoptimizer-arm.cc:401: // Compute caller's FP Read caller's FP from the previous frame, and set this frame's FP. https://chromiumcodereview.appspot.com/9265004/diff/4001/src/arm/deoptimizer-arm.cc#newcode413 src/arm/deoptimizer-arm.cc:413: // For the bottommost output frame the context can be gotten from the input Comment is just wrong: "A marker value is used in place of the context." https://chromiumcodereview.appspot.com/9265004/diff/4001/src/arm/deoptimizer-arm.cc#newcode436 src/arm/deoptimizer-arm.cc:436: // Number of incomming arguments. incomming => incoming https://chromiumcodereview.appspot.com/9265004/diff/4001/src/deoptimizer.cc File src/deoptimizer.cc (right): https://chromiumcodereview.appspot.com/9265004/diff/4001/src/deoptimizer.cc#newcode626 src/deoptimizer.cc:626: Address parameters_bot = parameters_top + parameters_size; _bottom reads better. https://chromiumcodereview.appspot.com/9265004/diff/4001/src/deoptimizer.h File src/deoptimizer.h (right): https://chromiumcodereview.appspot.com/9265004/diff/4001/src/deoptimizer.h#newcode439 src/deoptimizer.h:439: StackFrame::Type GetType() const { return type_; } Type and Kind are too generic (kind was before). Suggest GetFrameType/SetFrameType and GetCodeKind/SetCodeKind. https://chromiumcodereview.appspot.com/9265004/diff/4001/src/ia32/deoptimizer-ia32.cc File src/ia32/deoptimizer-ia32.cc (right): https://chromiumcodereview.appspot.com/9265004/diff/4001/src/ia32/deoptimizer-ia32.cc#newcode442 src/ia32/deoptimizer-ia32.cc:442: void Deoptimizer::DoComputeArgumentsAdaptorFrame(TranslationIterator* iterator, My suggestions about the comments in the ARM version mostly apply here as well. https://chromiumcodereview.appspot.com/9265004/diff/4001/src/ia32/deoptimizer-ia32.cc#newcode451 src/ia32/deoptimizer-ia32.cc:451: unsigned fixed_frame_size = 5 * kPointerSize; You could compute this as StandardFrameConstants::kFixedFrameSize + kPointerSize (with a comment that the extra pointer is the arg count (?)). Or better, add a named constant to ArgumentsAdaptorFrameConstants. https://chromiumcodereview.appspot.com/9265004/diff/4001/src/ia32/frames-ia32.h File src/ia32/frames-ia32.h (right): https://chromiumcodereview.appspot.com/9265004/diff/4001/src/ia32/frames-ia32.h#newcode100 src/ia32/frames-ia32.h:100: static const int kFixedFrameSize = 4; Currently unused, nice. Maybe a comment is needed here that we're counting the marker, context, caller fp, and caller pc as the fixed fields. https://chromiumcodereview.appspot.com/9265004/ -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
