Support inlining at call-sites with mismatched number of arguments.

Support inlining at call-sites with mismatched number of arguments.

Committed: https://code.google.com/p/v8/source/detail?r=10483

[Kevin Millikin (Chromium), 2012-01-19 10:26:34]

This approach seems straightforward.

https://chromiumcodereview.appspot.com/9265004/diff/1/src/hydrogen-instructio...
File src/hydrogen-instructions.h (right):

https://chromiumcodereview.appspot.com/9265004/diff/1/src/hydrogen-instructio...
src/hydrogen-instructions.h:1315: int arguments,
Make it sound like a number: "argument_count_" or something.

https://chromiumcodereview.appspot.com/9265004/diff/1/src/hydrogen.cc
File src/hydrogen.cc (right):

https://chromiumcodereview.appspot.com/9265004/diff/1/src/hydrogen.cc#newcode170
src/hydrogen.cc:170: last_environment_ =
last_environment()->LeaveInlined(drop_extra);
Maybe it's confusing to call the environment function by the same name as the
Hydrogen instruction (we don't do that for HEnterInlined/CopyForInlining).

You could use HLeaveInlined/DiscardInlined, or change CopyForInlining =>
EnterInlined.

[Vyacheslav Egorov (Chromium), 2012-01-23 19:49:34]

Hi Kevin,

The change is fully ready for your review. 

(it was rebased so difference from the first patch set is unfortunately
meaningless).

[Kevin Millikin (Chromium), 2012-01-24 00:08:54]

Hairy.  LGTM.

https://chromiumcodereview.appspot.com/9265004/diff/4001/src/arm/deoptimizer-...
File src/arm/deoptimizer-arm.cc (right):

https://chromiumcodereview.appspot.com/9265004/diff/4001/src/arm/deoptimizer-...
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-...
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-...
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-...
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-...
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#n...
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#ne...
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...
File src/ia32/deoptimizer-ia32.cc (right):

https://chromiumcodereview.appspot.com/9265004/diff/4001/src/ia32/deoptimizer...
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...
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...
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.

[Vyacheslav Egorov (Chromium), 2012-01-24 08:49:20]

Thanks for the review. Landed.

https://chromiumcodereview.appspot.com/9265004/diff/4001/src/arm/deoptimizer-...
File src/arm/deoptimizer-arm.cc (right):

https://chromiumcodereview.appspot.com/9265004/diff/4001/src/arm/deoptimizer-...
src/arm/deoptimizer-arm.cc:373: // The top address for the bottommost output
frame can be computed from
On 2012/01/24 00:08:54, Kevin Millikin wrote:
> 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." 

Done.

https://chromiumcodereview.appspot.com/9265004/diff/4001/src/arm/deoptimizer-...
src/arm/deoptimizer-arm.cc:391: // Compute caller's PC
On 2012/01/24 00:08:54, Kevin Millikin wrote:
> Read caller's PC from the previous frame.

Done.

https://chromiumcodereview.appspot.com/9265004/diff/4001/src/arm/deoptimizer-...
src/arm/deoptimizer-arm.cc:401: // Compute caller's FP
On 2012/01/24 00:08:54, Kevin Millikin wrote:
> Read caller's FP from the previous frame, and set this frame's FP.

Done.

https://chromiumcodereview.appspot.com/9265004/diff/4001/src/arm/deoptimizer-...
src/arm/deoptimizer-arm.cc:413: // For the bottommost output frame the context
can be gotten from the input
On 2012/01/24 00:08:54, Kevin Millikin wrote:
> Comment is just wrong:
> 
> "A marker value is used in place of the context."

Done.

https://chromiumcodereview.appspot.com/9265004/diff/4001/src/arm/deoptimizer-...
src/arm/deoptimizer-arm.cc:436: // Number of incomming arguments.
On 2012/01/24 00:08:54, Kevin Millikin wrote:
> incomming => incoming

Done.

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#ne...
src/deoptimizer.h:439: StackFrame::Type GetType() const { return type_; }
On 2012/01/24 00:08:54, Kevin Millikin wrote:
> Type and Kind are too generic (kind was before).   Suggest
> GetFrameType/SetFrameType and GetCodeKind/SetCodeKind.

Done.

https://chromiumcodereview.appspot.com/9265004/diff/4001/src/ia32/deoptimizer...
File src/ia32/deoptimizer-ia32.cc (right):

https://chromiumcodereview.appspot.com/9265004/diff/4001/src/ia32/deoptimizer...
src/ia32/deoptimizer-ia32.cc:442: void
Deoptimizer::DoComputeArgumentsAdaptorFrame(TranslationIterator* iterator,
On 2012/01/24 00:08:54, Kevin Millikin wrote:
> My suggestions about the comments in the ARM version mostly apply here as
well.

Done.

https://chromiumcodereview.appspot.com/9265004/diff/4001/src/ia32/deoptimizer...
src/ia32/deoptimizer-ia32.cc:451: unsigned fixed_frame_size = 5 * kPointerSize;
On 2012/01/24 00:08:54, Kevin Millikin wrote:
> 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.

Done.

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...
src/ia32/frames-ia32.h:100: static const int kFixedFrameSize    =  4;
On 2012/01/24 00:08:54, Kevin Millikin wrote:
> 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.

Done.