http://tinderbox.mozilla.org/showlog.cgi?log=SeaMonkey/1336654250.1336656302.26708.gz&fulltext=1 WINNT 5.2 comm-central-trunk build on 2012/05/10 05:50:50 { Factory.cpp e:\builds\slave\comm-cen-trunk-w32\build\mozilla\gfx\2d\Rect.h(91) : warning C4244: 'argument' : conversion from 'int32_t' to 'mozilla::gfx::Float', possible loss of data D:\msvs8\VC\INCLUDE\typeinfo(139) : warning C4275: non dll-interface class 'stdext::exception' used as base for dll-interface class 'std::bad_cast' D:\msvs8\VC\INCLUDE\typeinfo(160) : warning C4275: non dll-interface class 'stdext::exception' used as base for dll-interface class 'std::bad_typeid' D:\msvs8\VC\INCLUDE\intrin.h(944) : error C2733: second C linkage of overloaded function '_interlockedbittestandset' not allowed D:\msvs8\VC\INCLUDE\intrin.h(944) : see declaration of '_interlockedbittestandset' D:\msvs8\VC\INCLUDE\intrin.h(945) : error C2733: second C linkage of overloaded function '_interlockedbittestandreset' not allowed D:\msvs8\VC\INCLUDE\intrin.h(945) : see declaration of '_interlockedbittestandreset' e:/builds/slave/comm-cen-trunk-w32/build/mozilla/gfx/2d/Factory.cpp(251) : warning C4065: switch statement contains 'default' but no 'case' labels } https://tbpl.mozilla.org/?onlyunstarred=1&rev=b7b6565d12a0 https://hg.mozilla.org/mozilla-central/rev/b7b6565d12a0 As a guess, I assume this could be a "VC8 vs VC10" issue...

Sorry, I have no idea, I just performed the merge...

Perhaps bustage from Bug 732985

(In reply to Serge Gautherie (:sgautherie) from comment #0) > As a guess, I assume this could be a "VC8 vs VC10" issue... https://tbpl.mozilla.org/?tree=Thunderbird-Try&rev=fe8e10215e91 Fwiw, these SeaMonkey VC10 TB-Try builds succeeded.

(In reply to Serge Gautherie (:sgautherie) from comment #3) > (In reply to Serge Gautherie (:sgautherie) from comment #0) > > As a guess, I assume this could be a "VC8 vs VC10" issue... > > https://tbpl.mozilla.org/?tree=Thunderbird-Try&rev=fe8e10215e91 > > Fwiw, these SeaMonkey VC10 TB-Try builds succeeded. In that case, I'd LOVE if people could still use MSVC8 locally if this fix isn't too hard for that. Bas, is there an easy way to fix this, or do I need to bite the bullet and get my sorry bum to deploy the "build on MSVC2010" changes that are not fully written/tested yet for SeaMonkey machines?

https://tbpl.mozilla.org/?tree=Thunderbird-Try&rev=f61b5e6b3915 These SeaMonkey VC8 TB-Try builds failed: confirming compiler issue. *** (In reply to Justin Wood (:Callek) from comment #4) > In that case, I'd LOVE if people could still use MSVC8 locally if this fix > isn't too hard for that. Ftr, VC8+ is still supported, though MoCo use VC10 for official builds now.

MSVC2005 with SDK 7.1 cannot include intrin.h well. Workaround is - Don't include intrin.h if MSVC2005 (or 2008?) - Use inline assembler for cpuid

Comment on attachment 623933 [details] [diff] [review] fix - community is still building for MACOSX PPC. So don't use XP_MACOSX for SSE2 check. We should use GCC macro "__SSE2__" - Add workaround for MSVC2005

This is also busting xulrunner m-c nightlies on win32.

(In reply to Makoto Kato from comment #8) > Comment on attachment 623933 [details] [diff] [review] > fix > > - community is still building for MACOSX PPC. So don't use XP_MACOSX for > SSE2 check. We should use GCC macro "__SSE2__" > - Add workaround for MSVC2005 Doesn't GCC only define __SSE2__ whenc ompiling with -msse2?

(In reply to Bas Schouten (:bas) from comment #10) > (In reply to Makoto Kato from comment #8) > > Comment on attachment 623933 [details] [diff] [review] > > fix > > > > - community is still building for MACOSX PPC. So don't use XP_MACOSX for > > SSE2 check. We should use GCC macro "__SSE2__" > > - Add workaround for MSVC2005 > > Doesn't GCC only define __SSE2__ whenc ompiling with -msse2? To clarify, it's said that -msse2 is the default on OSX? But I don't really see that in the build logs.

On OS X, that is the default; gcc is equivalent to calling gcc -msse2 -fpmath=sse2 IIRC.

(In reply to Bas Schouten (:bas) from comment #11) > (In reply to Bas Schouten (:bas) from comment #10) > > (In reply to Makoto Kato from comment #8) > > > Comment on attachment 623933 [details] [diff] [review] > > > fix > > > > > > - community is still building for MACOSX PPC. So don't use XP_MACOSX for > > > SSE2 check. We should use GCC macro "__SSE2__" > > > - Add workaround for MSVC2005 > > > > Doesn't GCC only define __SSE2__ whenc ompiling with -msse2? > > To clarify, it's said that -msse2 is the default on OSX? But I don't really > see that in the build logs. Yes. gcc on XCode uses -msse2 by default. So it always define __SSE2__. Of course, x86-64 gcc (OSX, Linux and etc) sets __SSE2__ even if no -msse2.

Comment on attachment 623933 [details] [diff] [review] fix Review of attachment 623933 [details] [diff] [review]: ----------------------------------------------------------------- Approving this, I should reiterate that this -will- disable SSE2 for < MSVC10. Once we switch to Azure-for-content for D2D users very large images will be -slow- for < MSVC10 users.

I don't think that's too bad. Our official builds are VC10. Having the ability to compile the code with VC8 is important, even if the builds are suboptimal in perf. If someone is actually shipping code based on VC8, they can do the work to fix that if it's desired.

(In reply to Ted Mielczarek [:ted] from comment #16) > I don't think that's too bad. Our official builds are VC10. Having the > ability to compile the code with VC8 is important, even if the builds are > suboptimal in perf. If someone is actually shipping code based on VC8, they > can do the work to fix that if it's desired. ImageScalingSSE2.cpp cannot compile on VC8 due to _mm_castsi128_ps. So I think we should turn off SSE2 on ImageScaling to keep building using VC8 and file bug 756010. I believe that VC8 should be still target compiler for seamonkey and community, but it has some limitations due to old. Of course, if anyone fixes it, I am happy.

This is a VC8 bug; its intrin.h contains the wrong declarations for some of the functions. The bug was fixed in VC9. The fact that you're seeing the error means that you're already including the Platform SDK header that contains the correct declarations for those functions. I don't know whether the SDK header contains all the intrinsic declarations though.

Comment on attachment 623933 [details] [diff] [review] fix Review of attachment 623933 [details] [diff] [review]: ----------------------------------------------------------------- ::: gfx/2d/Factory.cpp @@ +96,5 @@ > #else > > +#if defined(_MSC_VER) && _MSC_VER >= 1600 && (defined(_M_IX86) || defined(_M_AMD64)) > +// MSVC 2005 or later supports __cpuid as intrin. > +// But it does't work on MSVC 2005 with SDK 7.1 (Bug 753772) I think this should either use "1500" or have a comment about MSVC 2008 case too.

marking fixed for status-firefox15