Bug 411379 introduced the use of __cpuid via #include <intrin.h>. MSVC71 hasn't got that header, causing the build to break. Random replacement that probably sucks: #ifdef __cplusplus extern "C" void __cpuid( int CPUInfo[4], int InfoType ); #endif /* __cplusplus */ void __cpuid( int CPUInfo[4], int InfoType ) { int my_eax, my_ebx, my_ecx, my_edx; __asm { mov eax, InfoType; cpuid mov my_eax, eax mov my_ebx, ebx mov my_ecx, ecx mov my_edx, edx } CPUInfo[0] = my_eax; CPUInfo[1] = my_ebx; CPUInfo[2] = my_ecx; CPUInfo[3] = my_edx; } (This fails on old 486s and earlier, where cpuid is an illegal instruction) (yes that code probably sucks)

I'll put back in the old code. Why is FF 3.0 running on MSVC71?

Remove the use of CPUID and the include of intrin.h.

Comment on attachment 299308 [details] [diff] [review] Undo CPUID usage. Setting r because us Songbird people like to get this in (but we're definitely installing shiny new vc8 buildbots too!) Thanks, mmoy :)

As I said in Bug 414072 VC2005 is the "official" compiler for Trunk. Does this change have any downsides mmoy?

Comment on attachment 299308 [details] [diff] [review] Undo CPUID usage. it would be better imho to just write cpuid() in asm and not revert all of this code.

The 486 doesn't support the instruction which is apparently the reason for a good chunk of the asm code in the MMX routine. Does FF 3.0 support the 486?

Okay, this instead attempts to implement __cpuid manually when not using MSVC8 or greater. Will cause SIGILL on 486 DX2 and older. Does that matter, or should I copy that larger block to check EFLAGS? This will be slower in the non-intrinsic path. Is that check for >= vc8 the right one? I do not have access to ICC to see if they have that header.

Comment on attachment 299352 [details] [diff] [review] implement __cpuid as mentioned in comment 0 changing reviewer based on conversation with pavlov in IRC. (he was also in favor of adding the eflags bit in)

(In reply to comment #8) > Will cause SIGILL on 486 DX2 and older. Does that matter, or > should I copy that larger block to check EFLAGS? I'm totally the wrong person to comment on this, but per http://en.wikipedia.org/wiki/486_DX2 this chip was obsoleted by the pentium series, and we apparently do not support anything older than win2k, which itself has a pentium processor as the minimum CPU requirement (http://en.wikipedia.org/wiki/Windows_2000#System_Requirements) - so it seems to me that not supporting the older chip would be OK.

I think you want #if _MSC_VER >= 1400 http://forums.microsoft.com/MSDN/ShowPost.aspx?PostID=163876&SiteID=1

Comment on attachment 299352 [details] [diff] [review] implement __cpuid as mentioned in comment 0 cancelling review, because - _MSC_VER test is wrong (it's 1400, and not hex) - not saving registers properly - while I'm at it, might as well put the 486 detection back.

Sorry for the bugspam and slowness; now probably works right :) According to dbradley, though, msvc8 doesn't generate any of the eflags checking (just a straight cpuid call).

It looks okay to me with the fix of the #endif comment. On the performance comment compared to intrinsics earlier, it's not an issue as this code is only called on the first decode.

If this looks good to you, please mark r+, thanks! :)

Mook, can you please try to get this into 1.9? I believe you'll need approval, but it's your patch... :-)

Comment on attachment 305409 [details] [diff] [review] fixed comment Err, crap, totally forgot about this, sorry. (I haven't built in a while) Should be pretty safe (preprocessed to not affect the shipping platforms at all). The code is mostly copied from previously working code too.

Comment on attachment 305409 [details] [diff] [review] fixed comment a1.9+=damons

Comment on attachment 305409 [details] [diff] [review] fixed comment stuart, can you rs= this since mmoy isn't a jpeg peer?

Checked in attachment 305409 [details] [diff] [review]: Checking in mozilla/jpeg/jdapimin.c; /cvsroot/mozilla/jpeg/jdapimin.c,v <-- jdapimin.c new revision: 3.11; previous revision: 3.10 done