Inline number to string conversion for string addition into BinaryOp(Stub).

Inline number to string conversion for string addition into BinaryOp(Stub).

This fixes a performance regression that was caused by converting the
BinaryOpStub to a Hydrogen code stub. It also fixes a leftover TODO wrt.
the handling of Number*String or String*Number versions of the stub.

R=rossberg@chromium.org

Committed: https://code.google.com/p/v8/source/detail?r=17290

[Benedikt Meurer, 2013-10-17 07:51:34]

Hey Andreas,
This fixes one of Oli's leftover TODOs. PTAL
CC'ed Oli for additional feedback.
-- Benedikt

[oliv, 2013-10-17 10:56:30]

https://codereview.chromium.org/27674002/diff/1/src/hydrogen.cc
File src/hydrogen.cc (right):

https://codereview.chromium.org/27674002/diff/1/src/hydrogen.cc#newcode1362
src/hydrogen.cc:1362: {
Please find a way to share the code with BuildSmiToString here...

[rossberg, 2013-10-17 11:02:18]

Looks good, except for the code duplication. Can we perhaps merge both functions
into one that takes a type (i.e., a lower bound) as an additional argument and
special-cases internally based on that?

https://codereview.chromium.org/27674002/diff/1/src/hydrogen.cc
File src/hydrogen.cc (right):

https://codereview.chromium.org/27674002/diff/1/src/hydrogen.cc#newcode1287
src/hydrogen.cc:1287: // Everything in here is safe wrt. observable side
effects.
Nit: this comment seems redundant, the next line says it all.

https://codereview.chromium.org/27674002/diff/1/src/hydrogen.cc#newcode1294
src/hydrogen.cc:1294: // Make the hash maks from the length of the number string
cache. It
Typo: mask

https://codereview.chromium.org/27674002/diff/1/src/hydrogen.cc#newcode1340
src/hydrogen.cc:1340: // Everything in here is safe wrt. observable side
effects.
Nit: as above

https://codereview.chromium.org/27674002/diff/1/src/hydrogen.cc#newcode1351
src/hydrogen.cc:1351: // Make the hash maks from the length of the number string
cache. It
Same typo

[Benedikt Meurer, 2013-10-18 06:53:05]

PTAL

https://codereview.chromium.org/27674002/diff/1/src/hydrogen.cc
File src/hydrogen.cc (right):

https://codereview.chromium.org/27674002/diff/1/src/hydrogen.cc#newcode1287
src/hydrogen.cc:1287: // Everything in here is safe wrt. observable side
effects.
On 2013/10/17 11:02:19, rossberg wrote:
> Nit: this comment seems redundant, the next line says it all.

Done.

https://codereview.chromium.org/27674002/diff/1/src/hydrogen.cc#newcode1294
src/hydrogen.cc:1294: // Make the hash maks from the length of the number string
cache. It
On 2013/10/17 11:02:19, rossberg wrote:
> Typo: mask

Done.

https://codereview.chromium.org/27674002/diff/1/src/hydrogen.cc#newcode1340
src/hydrogen.cc:1340: // Everything in here is safe wrt. observable side
effects.
On 2013/10/17 11:02:19, rossberg wrote:
> Nit: as above

Done.

https://codereview.chromium.org/27674002/diff/1/src/hydrogen.cc#newcode1351
src/hydrogen.cc:1351: // Make the hash maks from the length of the number string
cache. It
On 2013/10/17 11:02:19, rossberg wrote:
> Same typo

Done.

https://codereview.chromium.org/27674002/diff/1/src/hydrogen.cc#newcode1362
src/hydrogen.cc:1362: {
On 2013/10/17 10:56:30, oliv wrote:
> Please find a way to share the code with BuildSmiToString here...

Done.

[rossberg, 2013-10-21 12:06:17]

LGTM with nits

https://codereview.chromium.org/27674002/diff/77001/src/hydrogen.cc
File src/hydrogen.cc (right):

https://codereview.chromium.org/27674002/diff/77001/src/hydrogen.cc#newcode7849
src/hydrogen.cc:7849: right_type->Is(Type::String()))) {
Nit: should fit on previous line

https://codereview.chromium.org/27674002/diff/77001/src/hydrogen.h
File src/hydrogen.h (right):

https://codereview.chromium.org/27674002/diff/77001/src/hydrogen.h#newcode1238
src/hydrogen.h:1238: Handle<Type> type);
Nit: should fit on previous line

[Benedikt Meurer, 2013-10-21 12:09:42]

https://codereview.chromium.org/27674002/diff/77001/src/hydrogen.cc
File src/hydrogen.cc (right):

https://codereview.chromium.org/27674002/diff/77001/src/hydrogen.cc#newcode7849
src/hydrogen.cc:7849: right_type->Is(Type::String()))) {
On 2013/10/21 12:06:17, rossberg wrote:
> Nit: should fit on previous line

Done.

https://codereview.chromium.org/27674002/diff/77001/src/hydrogen.h
File src/hydrogen.h (right):

https://codereview.chromium.org/27674002/diff/77001/src/hydrogen.h#newcode1238
src/hydrogen.h:1238: Handle<Type> type);
On 2013/10/21 12:06:17, rossberg wrote:
> Nit: should fit on previous line

Done.

[Benedikt Meurer, 2013-10-21 12:42:16]

Committed patchset #5 manually as r17290 (presubmit successful).