Appreciate comments on this patch
-----Original Message-----
From: Xu, Samuel
Sent: Wednesday, November 17, 2010 1:48 PM
To: sandm...@daimi.au.dk; Siarhei Siamashka; Ma, Ling; Liu, Xinyun; Zhao,
Yakui; pixman@lists.freedesktop.org
Subject: RE: [Pixman] [ssse3]Optimization for fetch_scanline_x8r8g8b8
Hi, Soeren Sandmann and Siarhei Siamashka:
Glad to send out this refreshed patch to address points we discussed
for this SSSE3 patch.
In this new patch, we merged 32 bit and 64 bit asm code into unified
code (Macros are moved to header file, and make them more reusable and easily
to be read). We also fixed issue like CPU detection/git log/make file
checking/file name...etc. For MOVD, we simplified the backward copy code since
pervious code is too long and not gain obvious performance,
Thanks!
Samuel
-----Original Message-----
From: Xu, Samuel
Sent: Thursday, September 09, 2010 12:56 PM
To: 'sandm...@daimi.au.dk'; Siarhei Siamashka; Ma, Ling; Liu, Xinyun
Subject: RE: [Pixman] [ssse3]Optimization for fetch_scanline_x8r8g8b8
Hi, Soeren Sandmann and Siarhei Siamashka:
It is really a long discussion and thanks for your patience! I believe all of
us has same target to make pixman faster and better. Let's continue...
Merge 32 bit and 64 bit asm code into unified asm file is possible while has
pros and cons. E.g. I just tried Sun studio(my first experience on SS) and
found SS 12 can't expand Macro correctly, so failed to build some asm files
containing Macro. To make SS work, we originally want to remove existing
macros, while it seems we should use more macros.
Anyway, we will go ahead to following your 2 more suggestions: 1) movd adoption
2) unify 32 bit and 64 bit asm code via macro.
So the new patch will be shaped as:
1. Fix 64 bit CPU detection issue for MMX and SSE2 2. Add more comments for
git commit log 3. change SSSE3 intrinsics check to SSSE3 asm check in makefile
4. remove #include "pixman-combine32.h" and composite_over_8888_n_8888in
pixman-ssse3.c 5. ASM files changes:
1) Unify 32 bit and 64 bit asm code via macro
2) change asm file name to pixman-ssse3-x86-32-asm.S and
pixman-ssse3-x86-64-asm.S
3) change the asm function name to "composite_line_src_x888_8888_ssse3"
4) remove "defined(_M_AMD64))"
5) Comments in the assembly about the store forwarding
6) Use MOVD as Siarhei Siamashka's idea
Please raise the flag if those change are not completed or if Sun studio's
priority is higher than unified 32 bit and 64 bit code.
Thanks!
Samuel
-----Original Message-----
From: sandm...@daimi.au.dk [mailto:sandm...@daimi.au.dk]
Sent: Thursday, September 09, 2010 10:16 AM
To: Xu, Samuel
Cc: Siarhei Siamashka; pixman@lists.freedesktop.org; Liu, Xinyun; Ma, Ling
Subject: Re: [Pixman] [ssse3]Optimization for fetch_scanline_x8r8g8b8
"Xu, Samuel" <samuel...@intel.com> writes:
> Hi, Soeren Sandmann and Siarhei Siamashka:
>
> As a wrap of current discussion, combining you two's comments, can we assume
> this new patch of SSSE3 is ok?
> New patch might contains:
> 1. Fix 64 bit CPU detection issue for MMX and SSE2 2. Add more
> comments for git commit log 3. change SSSE3 intrinsics check to SSSE3
> asm check in makefile 4. remove #include "pixman-combine32.h" and
> composite_over_8888_n_8888in pixman-ssse3.c 5. ASM files changes:
> 1) change asm file name to pixman-ssse3-x86-32-asm.S and
> pixman-ssse3-x86-64-asm.S
> 2) change the asm function name to "composite_line_src_x888_8888_ssse3"
> 3) remove "defined(_M_AMD64))"
> 4) Comments in the assembly about the store forwarding
Such changes will certainly improve the patch.
> We know there are some discussion on asm code, e.g. MOVD, unify 32 bit
> and 64 bit code. While it won't introduce real defection. We still can
> put further change in next wave, to avoid current sticking.
No, we are not going to apply any patches that we know will need to be fixed
later, because I don't believe anyone is going to do that work.
It's fine to send a patch series if you think it's easier that way.
I am also interested in the answer to the question of whether this code was
generated by passing intrinsics through the Intel compiler.
> I am not sure for the issue of sun studio. Sun studio declares GNU asm
> compatibility, I am not sure whether it is 100% compatible. If issue
> is caused by Sun Studio itself, can we add #ifdef to avoid
> SSSE3 patch of Sun studio? In this case, how to determine Sun studio?
There are various approaches you can take with respect to unusual compilers
such as Sun Studio. The most common approach is to only enable the new
optimizations for the compilers that you are personally interested in, and
leave other compilers to people who are interested.
However, *you* introduced all this Sun Studio handling for SSSE3, which means
*you* need to be able to explain what it does, and justify why it is there.
Or to put it even more bluntly: Do not cut and paste code you don't understand
and can't test, and then ask me to rewrite the code for you until it is good
enough to go in.
Soren
_______________________________________________
Pixman mailing list
Pixman@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/pixman
