Fixing a performance bug introduced in r4581.

Fixing a performance bug introduced in r4581.

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

[Mads Ager (chromium), 2010-05-06 09:42:19]

http://codereview.chromium.org/1910005/diff/15001/16003
File src/arm/codegen-arm.cc (right):

http://codereview.chromium.org/1910005/diff/15001/16003#newcode8500
src/arm/codegen-arm.cc:8500: // Check that the third argument is a positive smi
not greater than the
Please restore comment.

http://codereview.chromium.org/1910005/diff/15001/16001
File src/ia32/codegen-ia32.cc (right):

http://codereview.chromium.org/1910005/diff/15001/16001#newcode10955
src/ia32/codegen-ia32.cc:10955: // Check that the third argument is a positive
smi not greater than the
The index should be less than the length. So please restore this comment.

http://codereview.chromium.org/1910005/diff/15001/16001#newcode10962
src/ia32/codegen-ia32.cc:10962: __ j(above, &runtime);
This actually should be above_equal. The index should be less than the subject
length.

http://codereview.chromium.org/1910005/diff/15001/16002
File src/x64/codegen-x64.cc (right):

http://codereview.chromium.org/1910005/diff/15001/16002#newcode8136
src/x64/codegen-x64.cc:8136: // Check that the third argument is a positive smi
not greater than the
Please restore old comment.

http://codereview.chromium.org/1910005/diff/15001/16002#newcode8142
src/x64/codegen-x64.cc:8142: __ j(above, &runtime);
above_equal is correct I think. The index should be less than the string length.

[SeRya, 2010-05-06 10:48:57]

The condition determines place where the corner case is handled. I checked both
ways (http://compute1.aar:9013/golem/r4596-v8-serya-perf-1-vs-4596-v8.html and
http://compute1.aar:9013/golem/r4596-v8-serya-perf-2-vs-4596-v8.html). Anyway
it's less important than wrong check for smi and should be measured
independently (and may be fixed separately).

http://codereview.chromium.org/1910005/diff/15001/16003
File src/arm/codegen-arm.cc (right):

http://codereview.chromium.org/1910005/diff/15001/16003#newcode8500
src/arm/codegen-arm.cc:8500: // Check that the third argument is a positive smi
not greater than the
On 2010/05/06 09:42:19, Mads Ager wrote:
> Please restore comment.

Done.

http://codereview.chromium.org/1910005/diff/15001/16001
File src/ia32/codegen-ia32.cc (right):

http://codereview.chromium.org/1910005/diff/15001/16001#newcode10955
src/ia32/codegen-ia32.cc:10955: // Check that the third argument is a positive
smi not greater than the
On 2010/05/06 09:42:19, Mads Ager wrote:
> The index should be less than the length. So please restore this comment.

Done.

http://codereview.chromium.org/1910005/diff/15001/16001#newcode10962
src/ia32/codegen-ia32.cc:10962: __ j(above, &runtime);
On 2010/05/06 09:42:19, Mads Ager wrote:
> This actually should be above_equal. The index should be less than the subject
> length.

Done.

[Mads Ager (chromium), 2010-05-06 11:18:25]

LGTM when 'le' has been changed to 'ls' on ARM.

http://codereview.chromium.org/1910005/diff/11004/27002
File src/arm/codegen-arm.cc (right):

http://codereview.chromium.org/1910005/diff/11004/27002#newcode8506
src/arm/codegen-arm.cc:8506: __ b(le, &runtime);
This should be 'ls' to use unsigned comparison!

[SeRya, 2010-05-06 12:50:36]

http://codereview.chromium.org/1910005/diff/11004/27002
File src/arm/codegen-arm.cc (right):

http://codereview.chromium.org/1910005/diff/11004/27002#newcode8506
src/arm/codegen-arm.cc:8506: __ b(le, &runtime);
On 2010/05/06 11:18:25, Mads Ager wrote:
> This should be 'ls' to use unsigned comparison!

Done.