Turn the NumberToStringStub into a hydrogen stub.

Turn the NumberToStringStub into a hydrogen stub.

This adds a BuildLookupNumberStringCache() in Hydrogen, which will
be used by the hydrogen version of StringAddStub, in addition to the
hydrogen version of NumberToStringStub.

R=mvstanton@chromium.org

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

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

[Benedikt Meurer, 2013-09-19 07:46:00]

Hey Michael,
Here's the final CL for the actual NumberToStringStub. Benchmarks seem to be
fine, no negative impact. PTAL
-- Benedikt

[Benedikt Meurer, 2013-09-20 07:32:00]

Somewhat more readable using the extended IfBuilder. :-)

[mvstanton, 2013-09-23 09:32:34]

Nice stuff! A few nits to look at and then lgtm.

https://codereview.chromium.org/23726041/diff/4001/src/code-stubs-hydrogen.cc
File src/code-stubs-hydrogen.cc (right):

https://codereview.chromium.org/23726041/diff/4001/src/code-stubs-hydrogen.cc...
src/code-stubs-hydrogen.cc:343: info()->MarkAsSavesCallerDoubles();
Comment this as to why you are doing it (the runtime call in
BuildNumberToString).

Also, I don't like that we use constant 0,1,2 for parameter numbers. In
ArrayConstructorStubBase, there is code like:

  // Parameters accessed via CodeStubGraphBuilder::GetParameter()
  static const int kConstructor = 0;
  static const int kPropertyCell = 1;

And then GetParameter calls in those stubs use the constants. Could you
similarly define a constant for NumberToStrings parameter?

https://codereview.chromium.org/23726041/diff/4001/src/code-stubs.h
File src/code-stubs.h (right):

https://codereview.chromium.org/23726041/diff/4001/src/code-stubs.h#newcode476
src/code-stubs.h:476: 
Here is a good place for something like:

  static const int kNumberParameter = 0;

https://codereview.chromium.org/23726041/diff/4001/src/hydrogen.cc
File src/hydrogen.cc (right):

https://codereview.chromium.org/23726041/diff/4001/src/hydrogen.cc#newcode1316
src/hydrogen.cc:1316: if_objectissmi.Else();
All this is really nice that you can use the IfBuilder and JoinContinuation!

https://codereview.chromium.org/23726041/diff/4001/src/hydrogen.cc#newcode9030
src/hydrogen.cc:9030: return
ast_context()->ReturnValue(BuildNumberToString(Pop()));
Paranoia: This code reminds me of the problem that in optimized C++
ast_context() may be evaluated before BuildNumberToString(Pop()). I would
separate that into two lines.

[Benedikt Meurer, 2013-09-23 10:53:30]

https://codereview.chromium.org/23726041/diff/4001/src/code-stubs-hydrogen.cc
File src/code-stubs-hydrogen.cc (right):

https://codereview.chromium.org/23726041/diff/4001/src/code-stubs-hydrogen.cc...
src/code-stubs-hydrogen.cc:343: info()->MarkAsSavesCallerDoubles();
On 2013/09/23 09:32:34, mvstanton wrote:
> Comment this as to why you are doing it (the runtime call in
> BuildNumberToString).
> 
> Also, I don't like that we use constant 0,1,2 for parameter numbers. In
> ArrayConstructorStubBase, there is code like:
> 
>   // Parameters accessed via CodeStubGraphBuilder::GetParameter()
>   static const int kConstructor = 0;
>   static const int kPropertyCell = 1;
> 
> And then GetParameter calls in those stubs use the constants. Could you
> similarly define a constant for NumberToStrings parameter?

Done.

https://codereview.chromium.org/23726041/diff/4001/src/code-stubs.h
File src/code-stubs.h (right):

https://codereview.chromium.org/23726041/diff/4001/src/code-stubs.h#newcode476
src/code-stubs.h:476: 
On 2013/09/23 09:32:34, mvstanton wrote:
> Here is a good place for something like:
> 
>   static const int kNumberParameter = 0;
> 

Done.

https://codereview.chromium.org/23726041/diff/4001/src/hydrogen.cc
File src/hydrogen.cc (right):

https://codereview.chromium.org/23726041/diff/4001/src/hydrogen.cc#newcode9030
src/hydrogen.cc:9030: return
ast_context()->ReturnValue(BuildNumberToString(Pop()));
On 2013/09/23 09:32:34, mvstanton wrote:
> Paranoia: This code reminds me of the problem that in optimized C++
> ast_context() may be evaluated before BuildNumberToString(Pop()). I would
> separate that into two lines.

Done.

[Benedikt Meurer, 2013-09-23 10:59:04]

Committed patchset #3 manually as r16874 (presubmit successful).

[Benedikt Meurer, 2013-09-24 09:31:26]

Committed patchset #3 manually as r16910 (presubmit successful).