Closed Bug 461202 Opened 11 years ago Closed 11 years ago

Turn on SSE2 pixman optimizations


(Core :: Graphics, defect, P3)

Windows XP





(Reporter: vlad, Assigned: jrmuizel)



(Keywords: fixed1.9.1, perf)


(1 file, 1 obsolete file)

Right now, we still have the SSE2 optimizations disabled in pixman for various reasons on various platforms.

For linux, we need bug 410509 (or similar) fixed; otherwise builds crash.

On win32, where that problem doesn't exist, I was seeing reftest failures the last time I tried this.
Flags: blocking1.9.1+
Shouldn't the __attribute__((__force_align_arg_pointer__)) in pixman deal with this? Or do we want to enable this for compilers that don't support __force_align_arg_pointer?
It should, but it didn't seem to -- it's entirely possible that the compiler I was trying with (what's on the tinderbox -- centos 5.1, maybe 5.2) doesn't support it.
Keywords: perf
It looks like we currently build with gcc 4.1 so we don't have
__force_align_arg_pointer. I don't know if bug 410509 is sufficient to
guarantee 128bit alignment for all the pixman calls...
While Bug 410509 is now fixed, is it now possible to turn on SSE2 in pixman without any risk? Or is there anything else to do before?
The reftests will almost certainly need updating, as the SSE2 code paths produce slighly different results for color operations, etc, due to rounding differences.
The reftests seem to pass for me. There should be no different results for color operations as the sse2 paths should produce identical results.
This includes MSVC and GCC's with __force_align_arg_pointer__.
Awesome, toss this on the try server?
I did some time ago. I don't remember there being any interesting results. I've started another run though.
Oh, ok -- just making sure that talos succeeded really.
The reftests now seem to be failing...
Nevermind, I was testing a broken build. I don't see any reason not to land this.
Keywords: checkin-needed
Keywords: checkin-needed
Target Milestone: --- → mozilla1.9.1b2
Closed: 11 years ago
Resolution: --- → FIXED
Backed out because of leak test failure on Linux.
Resolution: FIXED → ---
Target Milestone: mozilla1.9.1b2 → ---
Screw it; linux doesn't get to come along for this party.  It's irrelevant anyway, we won't really be using our pixman on linux for much.  Was win32 green and stuff?
Yep, Windows was all green.
I don't see how this patch could have caused a leak test failure as it shouldn't have caused any additional allocations, so either the it was false failure or it's pointing some other badness that this patch causes.
I don't think it caused leaks, it looks like it caused that box to go orange, which often happens when things crash.  So I'm guessing we're running into the SSE2 stack alignment issue, and the xpcom patch and the function alignment attribute isn't enough to fix it.
Only enable SSE2 on MSVC for now.
Attachment #347586 - Attachment is obsolete: true
Keywords: checkin-needed
Comment on attachment 349782 [details] [diff] [review]
Enable SSE2 on MSVC
[Checkin: Comment 20]
Attachment #349782 - Attachment description: Enable SSE2 on MSVC → Enable SSE2 on MSVC [Checkin: Comment 20]
Closed: 11 years ago11 years ago
Resolution: --- → FIXED
Whiteboard: [c-n: baking for 1.9.1]
Target Milestone: --- → mozilla1.9.2a1
(In reply to comment #20)
> (From update of attachment 349782 [details] [diff] [review])

Note that there was
patching file gfx/cairo/libpixman/src/
Hunk #1 succeeded at 69 with fuzz 2 (offset 2 lines).
In this case, you may want to check if the result is as wanted.!.
OS: All → Windows XP
Maybe Bug 447982 is of interest here.
I wonder if this is responsible for the big DHTML perf regression on Vista?
It shouldn't be, but the times are close.,787166,787165,787152
It could be. I'm surprised that any performance regressions wouldn't also show up on XP though.
Target Milestone: mozilla1.9.2a1 → mozilla1.9.1b3
Does this make any problems on Mac OS X because you only turned it on for windows? Or aren't there any advantages for Intel-Macs?
No real advantages on mac; we don't use pixman much there.  Only mild advantages on linux, and it causes problems due to gcc, so it's not enabled there either.
You need to log in before you can comment on or make changes to this bug.