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.

http://codereview.chromium.org/593110

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

Reply via email to