Start using String type info:

Start using String type info:

 * Improved string concatenation.

 * Fixed type inference in prefix/postfix count operations.

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

[fschneider, 2010-03-29 08:42:20]

LGTM with comments addressed.

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

http://codereview.chromium.org/1520001/diff/1/2#newcode1198
src/ia32/codegen-ia32.cc:1198: } else if (operands_type.IsString()) {
Actually the result of ADD is a string if either on both operands is a string.
So I think you could write here:

if (left.is_string() || right.is_string()) {
  return TypeInfo::String;
}

http://codereview.chromium.org/1520001/diff/1/2#newcode7137
src/ia32/codegen-ia32.cc:7137: old_value.set_type_info(TypeInfo::Number());
I'm not sure we can do that here. It is safe now because we don't make use of
the fact the old_value is a number. But we have not called ToNumber on the input
yet.

 I think you move this to after the deferred code exit where we are sure that
the returned value (old_value is the case of postfix) is a number.

Another improvement would be to avoid calling ToNumber if we know that the input
is a number but not a smi before.

[Vitaly Repeshko, 2010-03-29 10:29:31]

All comments addressed. Please take another look.


Thanks,
Vitaly

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

http://codereview.chromium.org/1520001/diff/1/2#newcode1198
src/ia32/codegen-ia32.cc:1198: } else if (operands_type.IsString()) {
On 2010/03/29 08:42:20, fschneider wrote:
> Actually the result of ADD is a string if either on both operands is a string.
> So I think you could write here:
> 
> if (left.is_string() || right.is_string()) {
>   return TypeInfo::String;
> }

Done.

http://codereview.chromium.org/1520001/diff/1/2#newcode7137
src/ia32/codegen-ia32.cc:7137: old_value.set_type_info(TypeInfo::Number());
On 2010/03/29 08:42:20, fschneider wrote:
> I'm not sure we can do that here. It is safe now because we don't make use of
> the fact the old_value is a number. But we have not called ToNumber on the
input
> yet.
> 
>  I think you move this to after the deferred code exit where we are sure that
> the returned value (old_value is the case of postfix) is a number.

I agree. It relies heavily on the fact the deferred code will convert it. So now
I keep old_value's type unknown in case it's not a number until the deferred
code returns.
 
> Another improvement would be to avoid calling ToNumber if we know that the
input
> is a number but not a smi before.

Good idea! Done. I'm passing the input type to the deferred code so that later
we can even call specialized ToNumber operations for different types.

[fschneider, 2010-03-29 11:05:18]

LGTM.

[Vitaly Repeshko, 2010-03-29 11:52:23]

Thanks for quick review! Submitted.


-- Vitaly