Pass the complete number type information into the GenericBinaryOpStub....

Pass the complete number type information into the GenericBinaryOpStub.

Currently we only pass a boolean parameter indicating whether
the input operands to the GenericBinaryOpStub are guaranteed
to be numbers or not. 

Instead we pass the complete number type as a parameters. This 
allows to use more precise type information for code generation
in the stub.

Also make the computation of the result type more precise and correct on both ia32 and x64.


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

[Lasse Reichstein, 2010-02-16 12:13:15]

What about the conditional operator - does it propagate types?
What about ARM?
LGTM.

http://codereview.chromium.org/593110/diff/3004/3005
File src/ia32/codegen-ia32.cc (right):

http://codereview.chromium.org/593110/diff/3004/3005#newcode1077
src/ia32/codegen-ia32.cc:1077: : NumberInfo::kNumber;
For and, you can even know that the result is a smi, if *either* operand is a
*positive* smi. Wonder if positiveness-analysis would be usefull.

http://codereview.chromium.org/593110/diff/3004/3006
File src/ia32/codegen-ia32.h (right):

http://codereview.chromium.org/593110/diff/3004/3006#newcode671
src/ia32/codegen-ia32.h:671: operands_type_(operands_type) {
Seems odd to pass only one operand type to a binary op stub. Can't you have
separate type information for each operand?

http://codereview.chromium.org/593110/diff/3004/3006#newcode710
src/ia32/codegen-ia32.h:710: static_cast<int>(operands_type_));
For debugging, you can have a function that converts Type bits to strings. You
have the code already in GenericBinaryOpStub::GetName

http://codereview.chromium.org/593110/diff/3004/3009
File src/number-info.h (right):

http://codereview.chromium.org/593110/diff/3004/3009#newcode52
src/number-info.h:52: return a >= kNumber;
Seems it should be ((a & kNumber) != 0)

http://codereview.chromium.org/593110/diff/3004/3007
File src/x64/codegen-x64.cc (right):

http://codereview.chromium.org/593110/diff/3004/3007#newcode8166
src/x64/codegen-x64.cc:8166: break;
This should be a function declared in numbertype.h and defined in codegen.cc.
It's identical across platforms.

http://codereview.chromium.org/593110/diff/3004/3008
File src/x64/codegen-x64.h (right):

http://codereview.chromium.org/593110/diff/3004/3008#newcode31
src/x64/codegen-x64.h:31: #include "number-info.h"
Don't include in a .h file. Just ensure that number-info.h is included before
codegen-x64.h wherever the latter is used.

[fschneider, 2010-02-16 12:55:59]

There are still operations (e.g. conditional, unary sub, unary not) where the
number type information is still missing. I'll add them as a separate change.

I'm not 100% sure about the implementation state of the virtual frame on ARM. We
do not perform the same virtual frame operations on ARM and also don't use the
Result class, so it may be hard to track type information there.

http://codereview.chromium.org/593110/diff/3004/3005
File src/ia32/codegen-ia32.cc (right):

http://codereview.chromium.org/593110/diff/3004/3005#newcode1077
src/ia32/codegen-ia32.cc:1077: : NumberInfo::kNumber;
On 2010/02/16 12:13:16, Lasse Reichstein wrote:
> For and, you can even know that the result is a smi, if *either* operand is a
> *positive* smi. Wonder if positiveness-analysis would be usefull.

Yes. I'll keep it simple for now :)

http://codereview.chromium.org/593110/diff/3004/3006
File src/ia32/codegen-ia32.h (right):

http://codereview.chromium.org/593110/diff/3004/3006#newcode671
src/ia32/codegen-ia32.h:671: operands_type_(operands_type) {
On 2010/02/16 12:13:16, Lasse Reichstein wrote:
> Seems odd to pass only one operand type to a binary op stub. Can't you have
> separate type information for each operand?

This is the weakest common type of both operands. I could not come up with a
more meaningful name.

http://codereview.chromium.org/593110/diff/3004/3006#newcode710
src/ia32/codegen-ia32.h:710: static_cast<int>(operands_type_));
On 2010/02/16 12:13:16, Lasse Reichstein wrote:
> For debugging, you can have a function that converts Type bits to strings. You
> have the code already in GenericBinaryOpStub::GetName

Done.

http://codereview.chromium.org/593110/diff/3004/3009
File src/number-info.h (right):

http://codereview.chromium.org/593110/diff/3004/3009#newcode52
src/number-info.h:52: return a >= kNumber;
On 2010/02/16 12:13:16, Lasse Reichstein wrote:
> Seems it should be ((a & kNumber) != 0)

Done.

http://codereview.chromium.org/593110/diff/3004/3007
File src/x64/codegen-x64.cc (right):

http://codereview.chromium.org/593110/diff/3004/3007#newcode8166
src/x64/codegen-x64.cc:8166: break;
On 2010/02/16 12:13:16, Lasse Reichstein wrote:
> This should be a function declared in numbertype.h and defined in codegen.cc.
> It's identical across platforms.

Done.

http://codereview.chromium.org/593110/diff/3004/3008
File src/x64/codegen-x64.h (right):

http://codereview.chromium.org/593110/diff/3004/3008#newcode31
src/x64/codegen-x64.h:31: #include "number-info.h"
On 2010/02/16 12:13:16, Lasse Reichstein wrote:
> Don't include in a .h file. Just ensure that number-info.h is included before
> codegen-x64.h wherever the latter is used.

Done. Moved into codegen.h. Hard to get out of codegen.h since this is included
in other .h files and lots of other places.