Added comments.
https://codereview.chromium.org/61623004/diff/30001/src/hydrogen-instructions.cc
File src/hydrogen-instructions.cc (right):
https://codereview.chromium.org/61623004/diff/30001/src/hydrogen-instructions.cc#newcode2866
src/hydrogen-instructions.cc:2866: return new(zone) Range(-16384,
16383);
Afaik signed integer goes from -2^15 to 2^15-1, which is -32768 to
32767.
I suggest introducing kMaxInt8, kMinInt8, etc. to globals.h and define
them either as -(1<<7) and (1<<7)-1 or -0x80 and 0x7f.
https://codereview.chromium.org/61623004/diff/30001/src/hydrogen-instructions.cc#newcode2869
src/hydrogen-instructions.cc:2869: return new(zone) Range(0, 65535);
Same here. Using a constant would be better.
https://codereview.chromium.org/61623004/diff/30001/src/hydrogen-instructions.cc#newcode2881
src/hydrogen-instructions.cc:2881: return new(zone) Range(0, 255);
While you are at it, this part can also be constantified.
https://codereview.chromium.org/61623004/diff/30001/src/ia32/lithium-ia32.cc
File src/ia32/lithium-ia32.cc (right):
https://codereview.chromium.org/61623004/diff/30001/src/ia32/lithium-ia32.cc#newcode2428
src/ia32/lithium-ia32.cc:2428: // Just force the value to be in eax and
we're safe here.
Don't we have the same issue with 16-bit accesses?
How do we actually manage for x64?
https://codereview.chromium.org/61623004/diff/30001/src/property-details.h
File src/property-details.h (right):
https://codereview.chromium.org/61623004/diff/30001/src/property-details.h#newcode137
src/property-details.h:137: return kind_ > other.kind_;
On 2013/11/07 09:01:06, Jakob wrote:
This simplification is not correct for e.g. kInteger8 vs. kUInteger8.
At least
please add ASSERTs to ensure this function isn't called for
representations it
can't handle.
Agree. It's probably better to special-case for representations below
kUInteger16.
https://codereview.chromium.org/61623004/diff/30001/src/x64/macro-assembler-x64.cc
File src/x64/macro-assembler-x64.cc (right):
https://codereview.chromium.org/61623004/diff/30001/src/x64/macro-assembler-x64.cc#newcode974
src/x64/macro-assembler-x64.cc:974: movw(dst, src);
Can we introduce ASSERTs for is_byte_register() for the 8bit and 16bit
case?
Also for ia32 of course.
https://codereview.chromium.org/61623004/
--
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
---
You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
For more options, visit https://groups.google.com/groups/opt_out.
