Support string add in crankshaft:

Support string add in crankshaft:

o The type recording binary stub got a new type for string + string.

o Added HStringAdd and LStringAdd based on the new type info.

o Started using HValue types to avoid unneccesary checks.

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

[Vitaly Repeshko, 2011-04-14 02:24:14]

This is ia32 for now. I'll make sure we at least don't get deopts on the other
architectures before submitting.


-- Vitaly

[Søren Thygesen Gjesse, 2011-04-14 07:04:12]

On 2011/04/14 02:24:14, Vitaly Repeshko wrote:
> This is ia32 for now. I'll make sure we at least don't get deopts on the other
> architectures before submitting.
> 
> 
> -- Vitaly

Drive by, how about the "drawing" in type-info.h? It is missing both this new
type and the oddball type. Should we remove it or update it?

[fschneider, 2011-04-14 10:26:47]

ia32 LGTM with comments below.

http://codereview.chromium.org/6852015/diff/1/src/hydrogen-instructions.h
File src/hydrogen-instructions.h (right):

http://codereview.chromium.org/6852015/diff/1/src/hydrogen-instructions.h#new...
src/hydrogen-instructions.h:1719: }
Good idea! We should look where else we can make use of inferred types.

http://codereview.chromium.org/6852015/diff/1/src/ia32/lithium-ia32.cc
File src/ia32/lithium-ia32.cc (right):

http://codereview.chromium.org/6852015/diff/1/src/ia32/lithium-ia32.cc#newcod...
src/ia32/lithium-ia32.cc:2006: LOperand* left = UseOrConstant(instr->left());
This should be UseOrConstantAtStart. Otherwise we can't allocate a register if
available since all are blocked because of the call.

http://codereview.chromium.org/6852015/diff/1/src/ic.cc
File src/ic.cc (right):

http://codereview.chromium.org/6852015/diff/1/src/ic.cc#newcode2029
src/ic.cc:2029: }
I think this is correct. Personally I like the approach better where we setup
the enum values in a way so that join becomes a & operator. Maybe that would be
a refactoring to do when cleaning up the duplicated type-info and HType code?

http://codereview.chromium.org/6852015/diff/1/src/ic.cc#newcode2030
src/ic.cc:2030: if (x >= y) return x;
Maybe have x > y here since you handle == cases already above.

[Vitaly Repeshko, 2011-04-14 21:07:18]

Florian, I addressed your comments and ported this to arm and x64.

On x64 I did a mini refactoring of the way constant operands are pushed. Having
a method that pushed both tagged and untagged values seemed unsafe. (We
discussed this with Slava). Lasse, it'd be great if you could have a look.


Thanks,
Vitaly

http://codereview.chromium.org/6852015/diff/1/src/ia32/lithium-ia32.cc
File src/ia32/lithium-ia32.cc (right):

http://codereview.chromium.org/6852015/diff/1/src/ia32/lithium-ia32.cc#newcod...
src/ia32/lithium-ia32.cc:2006: LOperand* left = UseOrConstant(instr->left());
On 2011/04/14 10:26:47, fschneider wrote:
> This should be UseOrConstantAtStart. Otherwise we can't allocate a register if
> available since all are blocked because of the call.

Done.

http://codereview.chromium.org/6852015/diff/1/src/ic.cc
File src/ic.cc (right):

http://codereview.chromium.org/6852015/diff/1/src/ic.cc#newcode2029
src/ic.cc:2029: }
On 2011/04/14 10:26:47, fschneider wrote:
> I think this is correct. Personally I like the approach better where we setup
> the enum values in a way so that join becomes a & operator. Maybe that would
be
> a refactoring to do when cleaning up the duplicated type-info and HType code? 

TRBinaryOpIC::TypeInfo has to stay dense for now because it's packed into the
stub key.

http://codereview.chromium.org/6852015/diff/1/src/ic.cc#newcode2030
src/ic.cc:2030: if (x >= y) return x;
On 2011/04/14 10:26:47, fschneider wrote:
> Maybe have x > y here since you handle == cases already above.

Done.

[Lasse Reichstein, 2011-04-15 05:12:25]

LGTM on the x64 stuff.

[fschneider, 2011-04-15 05:28:30]

LGTM.