LGTM, with comments.

http://codereview.chromium.org/119035/diff/1/5
File src/x64/assembler-x64.h (right):

http://codereview.chromium.org/119035/diff/1/5#newcode47
Line 47: static const unsigned int kUInt32Mask = 0xffffffff;
Please use the V8_UINT64_C() macro to define a 64-bit mask.  Relying on
the fact that it gets extended to 64 bits from an unsigned is confusing.

In fact, for consistency, copy the is_intn and is_uintn functions from
assembler.h, using 64-bit types.  Either instantiate is_uint32 directly,
or rename is_uintn to is_uint64n, since you can't overload on integer
type reliably, I think.

http://codereview.chromium.org/119035/diff/1/6
File src/x64/macro-assembler-x64.cc (right):

http://codereview.chromium.org/119035/diff/1/6#newcode56
Line 56: movq(dst, x, RelocInfo::NONE);
Let's put a warning here, at least until we see if people need this.  A
default Relocation mode of NONE is unsafe, unless we know that people
really will have 64-bit integers that are not coming from pointers.
Since the argument has integer type, rather than pointer type, it might
be OK to have that default mode here, but I was intending for all
relocation modes to be explicit.

http://codereview.chromium.org/119035

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

Reply via email to