Split off from bug 587936 comment 14. In gfx/thebes/Makefile.in: > ifeq (86,$(findstring 86,$(OS_TEST))) > ifdef __GNUC__ > gfxAlphaRecovery.$(OBJ_SUFFIX): MODULE_OPTIMIZE_FLAGS += -msse2 > endif This is rather dangerous. Setting -msse2 gives GCC permission to use sse2 instructions in the file wherever it pleases, even outside your CPUID guard. I'm not sure if recent versions of GCC will add SSE instructions at -Os (which is what we're currently using on Linux), but I know it will at -O3, which is what we're switching to in the near future. The only correct way that I'm aware of to use SSE intrinsics with GCC is to place them in a separate file. This can be fixed along with bug 585708.

Requires the patches from bug 585708 and bug 585818.

Comment on attachment 495336 [details] [diff] [review] Patch v1 Vlad, does this change look OK to you?

Fixing sunpro line in Makefile, hopefully.

Comment on attachment 495339 [details] [diff] [review] Patch v2 looks ok to me; hopefully we can drop non-SSE2 in the near future!

Comment on attachment 495339 [details] [diff] [review] Patch v2 Requesting approval. This should be a safe change -- it just moves code around. The benefits are making this vectorized code available on Linux 32 and being sure that gcc won't add SSE2 instructions into the unvectorized portion of the existing code.

This chain of patches passes try?

Also, given that this will actually cause us to fail to run on non-SSE2 processors, this should block final. AFAIK we still haven't started requiring SSE2.

(In reply to comment #7) > Also, given that this will actually cause us to fail to run on non-SSE2 > processors, this should block final. AFAIK we still haven't started requiring > SSE2. I don't think this will cause a crash on non-SSE2 processors unless GCC decides to vectorize the unvectorized loop in gfxAlphaRecovery.cpp. Contrast with bug 616778.

OK, well, in that case - have you run it on try so I can approve it? :)

Just pushed. :)

Well, the good news is that this doesn't break anything on Mac or Linux. The bad news is that it burns the Windows build. :) I'll have a closer look when I can; maybe Friday or the weekend. It's probably just something silly. http://tinderbox.mozilla.org/showlog.cgi?log=MozillaTry/1291754828.1291759934.2499.gz

My patch queue now passes on try. Requesting approval again.

Comment on attachment 495339 [details] [diff] [review] Patch v2 I'm not sure how it worked before, but MOZILLA_MAY_SUPPORT_SSE2 appears to be undefined on my system, and gfxAlphaRecoverySS2.cpp thus fails to compile.

(In reply to comment #14) > Comment on attachment 495339 [details] [diff] [review] > Patch v2 > > I'm not sure how it worked before, but MOZILLA_MAY_SUPPORT_SSE2 appears to be > undefined on my system, and gfxAlphaRecoverySS2.cpp thus fails to compile. Could you please give some details about your system and how you're building? You did a reconfigure, right?

Neil tells me that the failure above occurred with MSVC 7.1 (VS 2003). We're explicitly not supporting this version and iirc we knowingly break it elsewhere. I can dig up the bug where that discussion took place, if anyone is interested.