It's hard to tell in a makefile if you're targeting x86. configure.in tests that OS_TEST matches i?86, but I'm not aware of a way to do such wildcarding in Make. Patch forthcoming.

So it looks like we do this already, at least on Windows http://mxr.mozilla.org/mozilla-central/source/configure.in#2475

I'd be happy to move that code out of the windows-only section. I'd probably also want to set the variables to 1 as opposed to just defining them, since that lets you do ifneq (,$(_X86_)$(_AMD64_))

You'll also want to AC_SUBST and put these in autoconf.mk.in so that they're available in makefiles. AC_DEFINE only gets them available to c++.

Could we normalize TARGET_CPU to be "i386" when targeting x86 in general? I can't imagine that we actually have any Makefile code that wants to discriminate between i386 and i586.

That seems pretty sane to me.

How about normalizing OS_TEST to i386 instead of modifying TARGET_CPU?

Oh...there's apparently a CPU_ARCH variable which is set to x86 or x86_64, just like we want. Man, there's a lot of magic in here. I'll try it and see if it works.

IIRC CPU_ARCH isn't exported from configure. That isn't hard to fix though.

Expose CPU_ARCH to Makefiles.

What is the goal of INTEL_ARCHITECTURE? If you actually do need it you should be AC_SUBSTing it too.

(In reply to comment #11) > What is the goal of INTEL_ARCHITECTURE? If you actually do need it you should > be AC_SUBSTing it too. I totally thought I'd deleted that. On second thought, however, it *would* be nice to have a single "X86 or X86_64" variable; otherwise, you have to use |filter| on CPU_ARCH, which is just a pain. The whole point of this is to lower the activation energy required to use SIMD, after all. I'm not at all attached to the name, though.

Yeah, the name sucks. Do we support/plan to support any non-x86/ARM NEON SIMD instruction sets? If not, lets call it SIMD_[INTEL|ARM] or something similar.

(In reply to comment #13) > Yeah, the name sucks. Do we support/plan to support any non-x86/ARM NEON SIMD > instruction sets? If not, lets call it SIMD_[INTEL|ARM] or something similar. Well, that's not quite right, since the fact that we're targeting x86/x64 or ARM doesn't mean that the target supports SSE/NEON.

(In reply to comment #14) > (In reply to comment #13) > > Yeah, the name sucks. Do we support/plan to support any non-x86/ARM NEON SIMD > > instruction sets? If not, lets call it SIMD_[INTEL|ARM] or something similar. > > Well, that's not quite right, since the fact that we're targeting x86/x64 or > ARM doesn't mean that the target supports SSE/NEON. You're talking about an ARM/x86 compiler that doesn't support the relevant SIMD intrinsics, right? If that's the case, how is INTEL_ARCHITECTURE going to help?

> (In reply to comment #15) > You're talking about an ARM/x86 compiler that doesn't support the relevant SIMD > intrinsics, right? If that's the case, how is INTEL_ARCHITECTURE going to > help? I mean that SIMD_ARM is a misleading name in that it sounds like we can assume that the target supports NEON, when in fact it means that the *compiler* supports NEON. Perhaps the right thing is to define a variable which is 1 if the compiler supports a specific set of SIMD extensions for the target at hand. For instance, my understanding is that VS2003 doesn't support past SSE2, so we would define COMPILER_SUPPORTS_MMX, COMPILER_SUPPORTS_SSE, and COMPILER_SUPPORTS_SSE2 on that platform, but not COMPILER_SUPPORTS_SSE3. Of course, that's a lot more work, because we have to fill in that compatibility matrix...

Here's a patch which exposes CPU_ARCH and INTEL_ARCHITECTURE, as we discussed.

Oops; wrong version.

Comment on attachment 464980 [details] [diff] [review] Patch v3.1 WFM, but this all needs to be duplicated in js/src.

Here's v4 with khuey's r+.

Comment on attachment 465001 [details] [diff] [review] Patch v4 Requesting approval 2.0. This is a very low-risk change -- we're just exposing a few new variables -- and it will make the build much cleaner where we use x86 intrinsics (see e.g. bug 585538).

Hm. We may need to go the way of defining separate COMPILER_SUPPORTS_X variables. Bug 506430 needed a bustage fix for VS older than v1400 (2005?). I'm going to ask Neil about it after lunch, and will report back here.

1400 is 2005, yes. I don't really care about supporting anything older than that if it's a pain. VC 2003 is pretty old at this point.

You're OK having busted VS2003 builds?

I am at least, we build with 2005 on tinderbox. Idk how many devs use 2003.

Yes, we can simply unsupport 2003 on trunk.

(In reply to comment #22) > Hm. We may need to go the way of defining separate COMPILER_SUPPORTS_X > variables. Bug 506430 needed a bustage fix for VS older than v1400 (2005?). > I'm going to ask Neil about it after lunch, and will report back here. VC7.1 doesn't have __sse2_available. I don't know what replaced it on trunk.