488851 - Enable SSE2 for pixman

Status: RESOLVED FIXED

(Core :: Graphics, defect)

Description

[Makoto Kato [:m_kato]]

Due to bug 410509, SSE2 optimization for pixman is disable on GCC platform.
But this bug was already fixed at Oct 2008. So, we should enable SSE2 on GCC platform, too.

Comment 1

[Makoto Kato [:m_kato]]

Attachment: patch v1

Comment 2

[Jeff Muizelaar [:jrmuizel]]

We had something like this before in bug 461202, but it got backed out. We should try again.

Comment 3

[Makoto Kato [:m_kato]]

Attachment: patch v1.1

Tested on Linux x86/x86_64 and MacOS X (gcc platform).

Comment 4

[Makoto Kato [:m_kato]]

This patch causes that gcc may generate SSE2 code for pixman-mmx.c.

So I have to consider this situation, so I will send a review with new patch...

Comment 5

[Makoto Kato [:m_kato]]

Attachment: patch v2

Comment 6

[Jeff Muizelaar [:jrmuizel]]

Comment on attachment 376411 [details] [diff] [review]
patch v2

>diff -r ab664753cc72 gfx/cairo/libpixman/src/Makefile.in
>--- a/gfx/cairo/libpixman/src/Makefile.in    Fri May 08 13:46:36 2009 +0200
>+++ b/gfx/cairo/libpixman/src/Makefile.in    Sat May 09 00:08:59 2009 +0900
>@@ -69,6 +69,7 @@
> USE_MMX=1
> USE_SSE2=1
> MMX_CFLAGS=
>+SSE2_CFLAGS=
> endif
> endif
> ifeq (arm,$(findstring arm,$(OS_TEST)))
>@@ -81,11 +82,11 @@
> ifeq (86,$(findstring 86,$(OS_TEST)))
> USE_MMX=1
> MMX_CFLAGS=-mmmx -Winline
>-# See bug 410509 why we can't use SSE2 yet on linux
>-#USE_SSE2=1
>-#MMX_CFLAGS+=-msse -msse2
>+USE_SSE2=1
>+SSE2_CFLAGS=-msse2 -Winline 
> ifneq ($(MOZ_WIDGET_TOOLKIT),os2)
> MMX_CFLAGS+=--param inline-unit-growth=10000 --param large-function-growth=10000
>+SSE2_CFLAGS+=--param inline-unit-growth=10000 --param large-function-growth=10000

These lines should be vertically aligned like:
MMX_CFLAGS +=--param inline-unit-growth=10000 --param large-function-growth=10000
SSE2_CFLAGS+=--param inline-unit-growth=10000 --param large-function-growth=10000

Other than that, it looks good to me.

Comment 7

[Makoto Kato [:m_kato]]

backout checkin.

But no symbols set up on Linux leak test. I cannot analyze where is failure due to no stack.
http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1242397437.1242398983.16275.gz

I will investigate this again.

Comment 8

[Makoto Kato [:m_kato]]

segfault in fbcomposeSetupSSE2.  Maybe, it is by alignment access.

Comment 9

[Makoto Kato [:m_kato]]

Attachment: patch v3

Comment 10

[Makoto Kato [:m_kato]]

Attachment: patch v4

Comment 11

[Makoto Kato [:m_kato]]

Comment on attachment 432415 [details] [diff] [review]
patch v4

old GCC such as 4.1 doesn't support align arg pointer.  If it is old, don't compile SSE2 code.

Comment 12

[Justin Lebar (not reading bugmail)]

Looks like this was landed (c4b8a7e21ec9) and backed out (8cf7bc6629a7).  What's its current status?

Comment 13

[Makoto Kato [:m_kato]]

(In reply to comment #12)
> Looks like this was landed (c4b8a7e21ec9) and backed out (8cf7bc6629a7). 
> What's its current status?

Ahh, I forgot pushing this!.  8cf7bc6629a7 was gcc 4.1.x bug that past tinderbox used.  New patch checks gcc really has alignment support.

Also, current tinderbox is gcc4.3 or 4.4.

Comment 14

[Joe Drew (not getting mail)]

Comment on attachment 432415 [details] [diff] [review]
patch v4

Very short leash on this one - push it, but if it bounces, it's out.

Comment 15

[Makoto Kato [:m_kato]]

http://hg.mozilla.org/mozilla-central/rev/6d435ea11c55