Include 64-bit pointer and constant types in include/v8.h....

Include 64-bit pointer and constant types in include/v8.h.
Make some definitions in globals.h 64-bit safe

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

[Lasse Reichstein, 2009-05-04 12:43:11]

LGTM, with comments.

http://codereview.chromium.org/100336/diff/1/3
File src/globals.h (right):

http://codereview.chromium.org/100336/diff/1/3#newcode54
Line 54: #ifdef _WIN32
Is _WIN32 correct for 64 bits? Shouldn't it rather be _MSC_VER?

http://codereview.chromium.org/100336/diff/1/3#newcode58
Line 58: #define UINT64_C(x)  (x ## ULL)
Or should these be by architecture (so ##UL on GCC/LP64 and #UUL on GCC/ILP32)?

http://codereview.chromium.org/100336/diff/1/3#newcode96
Line 96: const intptr_t kObjectAlignmentMask = (ONE << kObjectAlignmentBits) -
1;
How about
 ~(-1 << kObjectAlignmentBits)
instead? All the masks are int values anyway, so the calculation can be done on
ints.

[William Hesse, 2009-05-04 13:36:36]

http://codereview.chromium.org/100336/diff/1/3
File src/globals.h (right):

http://codereview.chromium.org/100336/diff/1/3#newcode54
Line 54: #ifdef _WIN32
On 2009/05/04 12:43:11, Lasse Reichstein wrote:
> Is _WIN32 correct for 64 bits? Shouldn't it rather be _MSC_VER?

Done.

http://codereview.chromium.org/100336/diff/1/3#newcode58
Line 58: #define UINT64_C(x)  (x ## ULL)
On 2009/05/04 12:43:11, Lasse Reichstein wrote:
> Or should these be by architecture (so ##UL on GCC/LP64 and #UUL on
GCC/ILP32)?

I don't know that these are defined.

http://codereview.chromium.org/100336/diff/1/3#newcode96
Line 96: const intptr_t kObjectAlignmentMask = (ONE << kObjectAlignmentBits) -
1;
On 2009/05/04 12:43:11, Lasse Reichstein wrote:
> How about
>  ~(-1 << kObjectAlignmentBits)
> instead? All the masks are int values anyway, so the calculation can be done
on
> ints.

This change is removed, so that masks of more than 32 bits will give a compiler
error, for portability.

[Dean McNamee, 2009-05-04 16:09:52]

http://codereview.chromium.org/100336/diff/11/1006
File src/globals.h (right):

http://codereview.chromium.org/100336/diff/11/1006#newcode59
Line 59: #define INT64_C(x)   (x ## LL)
We should use our own names here.

Also, the stdint code takes in to account 64 bit:

# if __WORDSIZE == 64
#  define __INT64_C(c)  c ## L
#  define __UINT64_C(c) c ## UL
# else
#  define __INT64_C(c)  c ## LL
#  define __UINT64_C(c) c ## ULL
# endif

Looks like LL should be 64bit on both though.

[iposva, 2009-05-04 17:00:55]

This is a general comment about using V8_ARCH_X64 as the 64-bit marker while
porting and not only a particular comment about Bill's change.

Please clean this up now and stick to the clean definition before it gets a lot
harder to deal with. Some ideas: Use the automatic defines __WORDSIZE or
__LP64__ or something that is defined in one place inside globals.h or v8.h
based on the compiler automatic defines:
#if defined(__LP64__) || (__LLP64__)
#define V8_64BIT 1
#endif

Thanks!
-Ivan

http://codereview.chromium.org/100336/diff/11/1006
File src/globals.h (right):

http://codereview.chromium.org/100336/diff/11/1006#newcode87
Line 87: #ifdef V8_ARCH_X64
There are potentially other 64-bit architectures outside x64. Please do not
"improve" the code for 64-bit cleanliness while making it harder to port to
other architectures. This will serve as a disservice in the long time. Thanks!

http://codereview.chromium.org/100336/diff/11/1006#newcode124
Line 124: #ifdef V8_ARCH_X64
ditto!