I have a patch out for review. I will clean it up tomorrow. Feel free to chime in on how you want the naming.
http://codereview.chromium.org/99355 I think something like ARCH_CPU_X64 (I guess we're going with x64 and not x86-64?), and also ARCH_CPU_32_BITS and ARCH_CPU_64_BITS is good. I can change those to V8_ARCH if people like that better. On Mon, May 4, 2009 at 9:13 PM, Lasse R.H. Nielsen <[email protected]> wrote: > I absolutely agree. Any significant information in the system must be > defined in exactly one place. Preferably somewhere under our control. > We should be exact on what it means, though. In this case it's used to > select our data/heap model (which should be compatible to the compiler's > data model, but not identical - we will use the same heap model for both > GCC's LP64 and MSVC's LLP64 data models). As such, it is different from the > V8_ARCH_* flags, which select the target back-end (we use the same heap > model for IA32 and ARM architectures). Theoretically, we could have more > than one heap model for the same architecture (e.g., full 64-bit or 64-bit > with compressed pointers (or some other optimization we might make), or > even, hopefully not, a 32-bit heap for the 64-bit backend). > With that reasoning, the LPC64/LLPC64 flags could just as well use a > compiler defined flag (if one exists - and yes, that's a retraction of my > former opinion :). The constant macros depend on the choice of compiler and > choice of data model for the compiler, both something that we know, but that > the compiler also knows. > A single flag, e.g., V8_HEAP_64BIT, would be a better selector for the > 64-bit pointer/heap model (vs. the current 32-bit model). > /L > On Mon, May 4, 2009 at 7:00 PM, <[email protected]> wrote: >> >> 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! >> >> http://codereview.chromium.org/100336 > > > > -- > Lasse R.H. Nielsen > [email protected] > 'Faith without judgement merely degrades the spirit divine' > --~--~---------~--~----~------------~-------~--~----~ v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev -~----------~----~----~----~------~----~------~--~---
