http://codereview.chromium.org/99355/diff/1/2 File src/codegen.h (right):
http://codereview.chromium.org/99355/diff/1/2#newcode79 Line 79: #if ARCH_CPU_X86 On 2009/05/05 08:57:59, Lasse Reichstein wrote: > Should it be #if defined(ARCH_CPU_X86) ? We learned this lesson from chrome, where we still do it the more annoying way. - In C++ an undefined macro will evaluate to 0. - #if defined() is more annoying. - It is sometimes useful to be able to set things to 0, from the build system, etc. I think it makes things easier to read, but if people are really worried about it, we can move everything to defined() :( > Calling it CPU isn't exactly right. What we are selecting is our backend, which > is correclated with the CPU, but not identical to it. > CPU seems too specific (e.g., Intel Xeon EM64T Irwindale 3.4GHz is a CPU, but > X64 or IA32 are architectures shared by several CPUs). I would prefer just > ARCH_IA32, or even better: V8_ARCH_IA32 :) > And use the traditional names for wintel architectures: IA32 and X64. http://codereview.chromium.org/99355/diff/1/2#newcode83 Line 83: #elif ARCH_CPU_ARMEL On 2009/05/05 04:44:27, iposva wrote: > I like this style better, especially if it ends with a > #else > #error Unknown architecture. > #endif > This makes it a lot easier to find places to patch when implementing a new > platform. We will already get a #error since everything is included globals.h. I'd rather keep that in one place, and not have a copy/pasted error everywhere. http://codereview.chromium.org/99355/diff/1/7 File src/jsregexp.h (right): http://codereview.chromium.org/99355/diff/1/7#newcode42 Line 42: #else On 2009/05/05 08:57:59, Lasse Reichstein wrote: > I actually prefer to not have a default case, so that a completely unknown > architecture generates an error. > I.e., test for all the known ones, and generate an error if none match. Completely unknown architectures are already going to cause an error by including globals.h. I would rather not have a #error with the same error message every place where we need to select based on the architecture. http://codereview.chromium.org/99355 --~--~---------~--~----~------------~-------~--~----~ v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev -~----------~----~----~----~------~----~------~--~---
