Fix xssue 492: ARM debug crash: mozilla/ecma/FunctionObjects/15.3.1.1-3...

Fix xssue 492: ARM debug crash: mozilla/ecma/FunctionObjects/15.3.1.1-3

When the number of parameters times 4 (kPointerSize) to a function cannot be encoded in 12 bits the return sequence gets one more instruction. Changed the assertion to check for this case.

BUG=http://code.google.com/p/v8/issues/detail?id=492
TEST=test/mjsunit/regress/regress-492.js

Committed: http://code.google.com/p/v8/source/detail?r=3211

[Erik Corry, 2009-11-03 17:48:52]

LGTM with a regression test.

http://codereview.chromium.org/354028/diff/1/2
File src/arm/codegen-arm.cc (right):

http://codereview.chromium.org/354028/diff/1/2#newcode321
Line 321: //masm_->BlockConstPoolBefore(masm_->pc_offset() + 4 *
kJSReturnSequenceLength);
Please don't commit this commented out code.

[Erik Corry, 2009-11-03 17:56:40]

Oops!  Laurent Desnogues points out that this is wrong.  The correct test is not
is_int12.  You need to access or duplicate the test in fits_shifter from
assembler-arm.cc to see whether it can be encoded in the 12 bit immediate field
of an ARM instruction.

[Erik Corry, 2009-11-03 17:56:41]

Oops!  Laurent Desnogues points out that this is wrong.  The correct test is not
is_int12.  You need to access or duplicate the test in fits_shifter from
assembler-arm.cc to see whether it can be encoded in the 12 bit immediate field
of an ARM instruction.

[Søren Thygesen Gjesse, 2009-11-03 22:57:33]

Erik, please take another look.

Fixed the check for whether the immediate can be encoded as part of the add
instruction - Laurent thanks for spotting this. I probably looked at addrmod2
encoding instead of addrmod1 encoding when using is_uint12().

Added a regression test using a argument count interval arround 2^8.

http://codereview.chromium.org/354028/diff/1/2
File src/arm/codegen-arm.cc (right):

http://codereview.chromium.org/354028/diff/1/2#newcode321
Line 321: //masm_->BlockConstPoolBefore(masm_->pc_offset() + 4 *
kJSReturnSequenceLength);
On 2009/11/03 17:48:52, Erik Corry wrote:
> Please don't commit this commented out code.

Sorry, removed.

[Erik Corry, 2009-11-04 09:33:09]

LGTM

http://codereview.chromium.org/354028/diff/7005/10003
File src/arm/assembler-arm.h (right):

http://codereview.chromium.org/354028/diff/7005/10003#newcode686
Line 686: bool ImmediateFitsAddInstruction(int32_t imm32);
It's not just add instructions.  Almost all ARM instructions take an immediate
with these restrictions.  So the name should be more general.

[Søren Thygesen Gjesse, 2009-11-04 09:54:38]

http://codereview.chromium.org/354028/diff/7005/10003
File src/arm/assembler-arm.h (right):

http://codereview.chromium.org/354028/diff/7005/10003#newcode686
Line 686: bool ImmediateFitsAddInstruction(int32_t imm32);
On 2009/11/04 09:33:09, Erik Corry wrote:
> It's not just add instructions.  Almost all ARM instructions take an immediate
> with these restrictions.  So the name should be more general.

Changed name to ImmediateFitsAddrMode1Instruction and added a comment that add
is an addressing mode 1 instruction.