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.

http://codereview.chromium.org/1520001

--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev

To unsubscribe from this group, send email to v8-dev+unsubscribegooglegroups.com or reply 
to this email with the words "REMOVE ME" as the subject.

Reply via email to