<div dir="ltr">Since most of these functions force inlining, argument passing by pointer or by value<br>should not matter, modulo compiler inability to do the right thing.<br><br>I think that there are no issues from changing some args from values to regs<br>
as long as GCC still manages to generate decent code. From what I recall, this is not<br>the case and some reasonable recent version of it should do the trick.<br><br>Frédéric, do you mind testing that for gcc? <br><br>Thanks,<br>
Rodrigo<br><br><div class="gmail_quote">On Mon, Jul 21, 2008 at 6:19 AM, Antoine Azar <<a href="mailto:cairo@antoineazar.com">cairo@antoineazar.com</a>> wrote:<br><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">
Yep, I've tried everything on my side (playing with compiler switches,<br>
setting the function parameters as const references, wrapping in structs,<br>
changing stack size, etc) and didn't have more luck than Fred.<br>
<br>
VC++ always borks starting the fourth __m128i parameter only. The three<br>
first ones are always ok. It seems this is a limitation of VC++, that it'll<br>
only handle a "pass by register" of up to three SSE intrinsics, and not one<br>
more.<br>
<br>
I don't think sending pointers would be much of an issue: this is already<br>
done at many other places in the code, and it'll avoid the overhead of<br>
passing by value 16 byte data structures.<br>
Otherwise, we can try rewriting functions so they always use at most 3 SSE<br>
intrinsics that are passed by value, but that might be a pain.<br>
<br>
I'd be curious to see how the Intel Compiler handles this.<br>
<br>
Antoine<br>
<div><div></div><div class="Wj3C7c"><br>
<br>
> -----Original Message-----<br>
> From: <a href="mailto:cairo-bounces@cairographics.org">cairo-bounces@cairographics.org</a><br>
> [mailto:<a href="mailto:cairo-bounces@cairographics.org">cairo-bounces@cairographics.org</a>] On Behalf Of Frédéric Plourde<br>
> Sent: Sunday, July 20, 2008 10:03 PM<br>
> To: <a href="mailto:cairo@cairographics.org">cairo@cairographics.org</a><br>
> Subject: [cairo] Problem compiling pixman-sse.c under<br>
> Win32,blame it on MSVC !!<br>
><br>
> Hi !<br>
><br>
> Vlad had emailled me with this problem compiling pixman<br>
> sith SSE2 stuff enabled.<br>
> In fact, here are the errors that we got :<br>
><br>
> pixman-sse.c<br>
> pixman-sse.c(142) : error C2719: 'xmm3': formal parameter with<br>
> __declspec(align('16')) won't be aligned<br>
> pixman-sse.c(195) : error C2719: 'alphaHi': formal parameter with<br>
> __declspec(align('16')) won't be aligned<br>
> pixman-sse.c(210) : error C2719: 'alphaDstHi': formal parameter with<br>
> __declspec(align('16')) won't be aligned<br>
> pixman-sse.c(210) : error C2719: 'dstLo': formal parameter with<br>
> __declspec(align('16')) won't be aligned<br>
> pixman-sse.c(210) : error C2719: 'dstHi': formal parameter with<br>
> __declspec(align('16')) won't be aligned<br>
> pixman-sse.c(210) : error C2719: 'alphaSrcLo': formal parameter with<br>
> __declspec(align('16')) won't be aligned<br>
> pixman-sse.c(210) : error C2719: 'alphaSrcHi': formal parameter with<br>
> __declspec(align('16')) won't be aligned<br>
> pixman-sse.c(246) : error C2719: 'alphaHi': formal parameter with<br>
> __declspec(align('16')) won't be aligned<br>
> pixman-sse.c(276) : error C2719: 'alphaHi': formal parameter with<br>
> __declspec(align('16')) won't be aligned<br>
> pixman-sse.c(276) : error C2719: 'maskLo': formal parameter with<br>
> __declspec(align('16')) won't be aligned<br>
> pixman-sse.c(276) : error C2719: 'maskHi': formal parameter with<br>
> __declspec(align('16')) won't be aligned<br>
> pixman-sse.c(290) : warning C4133: 'function' : incompatible<br>
> types - from '__m128i *' to 'const char *'<br>
> pixman-sse.c(296) : warning C4133: 'function' : incompatible<br>
> types - from '__m128i *' to 'const char *'<br>
> pixman-sse.c(371) : error C2719: 'alphaSrc': formal parameter with<br>
> __declspec(align('8')) won't be aligned<br>
> pixman-sse.c(398) : error C2719: 'dst': formal parameter with<br>
> __declspec(align('8')) won't be aligned<br>
> pixman-sse.c(4432) : error C2275: '__m128i' : illegal use of<br>
> this type as an expression<br>
> D:\msvs8\VC\INCLUDE\emmintrin.h(45) : see declaration<br>
> of '__m128i'<br>
> pixman-sse.c(4432) : error C2146: syntax error : missing ';'<br>
> before identifier 'xmm0'<br>
> pixman-sse.c(4432) : error C2065: 'xmm0' : undeclared identifier<br>
> pixman-sse.c(4432) : error C2065: 'xmm1' : undeclared identifier<br>
> pixman-sse.c(4432) : error C2065: 'xmm2' : undeclared identifier<br>
> pixman-sse.c(4432) : error C2065: 'xmm3' : undeclared identifier<br>
> pixman-sse.c(4434) : error C2440: '=' : cannot convert from<br>
> '__m128i' to 'int'<br>
> pixman-sse.c(4435) : error C2440: '=' : cannot convert from<br>
> '__m128i' to 'int'<br>
> pixman-sse.c(4436) : error C2440: '=' : cannot convert from<br>
> '__m128i' to 'int'<br>
> pixman-sse.c(4437) : error C2440: '=' : cannot convert from<br>
> '__m128i' to 'int'<br>
> pixman-sse.c(4439) : error C2440: 'function' : cannot convert<br>
> from 'int'<br>
> to '__m128i'<br>
> pixman-sse.c(4439) : warning C4024: 'save128Aligned' :<br>
> different types for formal and actual parameter 2<br>
> pixman-sse.c(4440) : error C2440: 'function' : cannot convert<br>
> from 'int'<br>
> to '__m128i'<br>
> pixman-sse.c(4440) : warning C4024: 'save128Aligned' :<br>
> different types for formal and actual parameter 2<br>
> pixman-sse.c(4441) : error C2440: 'function' : cannot convert<br>
> from 'int'<br>
> to '__m128i'<br>
> pixman-sse.c(4441) : warning C4024: 'save128Aligned' :<br>
> different types for formal and actual parameter 2<br>
> pixman-sse.c(4442) : error C2440: 'function' : cannot convert<br>
> from 'int'<br>
> to '__m128i'<br>
> pixman-sse.c(4442) : warning C4024: 'save128Aligned' :<br>
> different types for formal and actual parameter 2<br>
><br>
><br>
> We found the following :<br>
><br>
><br>
> for every C2719 errors, the solution is to avoid passing<br>
> __m128i values directly, because MSVC just doesn't allow<br>
> function parameters to be of types declared with the<br>
> "__declspec(align('#'))" modifier (this is the case of<br>
> __m128i). See:<br>
> <a href="http://msdn.microsoft.com/en-us/library/373ak2y1%28VS.80%29.aspx" target="_blank">http://msdn.microsoft.com/en-us/library/373ak2y1(VS.80).aspx</a><br>
><br>
> More problematic is the behaviour of MSVC under these<br>
> assumptions... in fact, try removing ONLY the xmm3 function<br>
> parameter at line #142 in pixman-sse.c (and by commenting<br>
> references to xmm3 inside the function, of course), it will<br>
> compile. (!!). I've went through different forums for<br>
> developpers and it seems there are still some bugs with those<br>
> intrinsics in MSVC8. Very dissapointing!<br>
><br>
> So my recommandation to misters Tupinamba and Kumpera would<br>
> be to pass POINTERS to those function parameters instead...<br>
> as :<br>
><br>
> static inline __m128i<br>
> pack565_4x128_128 (__m128i *xmm0, __m128i *xmm1, __m128i<br>
> *xmm2, __m128i *xmm3) {<br>
> __m128i lo, hi;<br>
><br>
> lo = _mm_packus_epi16 (pack565_2x128_128 ( *xmm0, *xmm1 ),<br>
> _mm_setzero_si128 ());<br>
> hi = _mm_packus_epi16 (_mm_setzero_si128 (),<br>
> pack565_2x128_128 ( *xmm2, *xmm3 ));<br>
><br>
> return _mm_or_si128 (lo, hi);<br>
> }<br>
><br>
> compiles perfectly.<br>
> However, this would maybe imply significant impacts on<br>
> performance... it should be addressed and profiled..<br>
><br>
><br>
> ******<br>
> Besides, for the C2275 error, it's even more embarassing for<br>
> microsoft... it's another case of MSVC being very capricious.<br>
> In some cases, with aligned variables, MSVC doesn't allow us<br>
> to declare<br>
> those variable in the middle of the function... We then need<br>
> to move the<br>
> declaration earlier.<br>
> see, at line 4432,<br>
><br>
> __m128i xmm0, xmm1, xmm2, xmm3; needs to be moved up a<br>
> little... as :<br>
><br>
> while (w >= 64)<br>
> {<br>
> __m128i xmm0, xmm1, xmm2, xmm3;<br>
><br>
> /* 128 bytes ahead */<br>
> cachePrefetch (((__m128i*)s) + 8);<br>
> cachePrefetch (((__m128i*)d) + 8);<br>
><br>
> compiles now...<br>
> ****************<br>
><br>
> So here it is... a patch with those changes in pixman-sse.c<br>
> I'd like you guys (especcially André) to look that over and<br>
> tell me what<br>
> you think. It's a first draft, but it compiles under all 3<br>
> major platforms.<br>
><br>
> Best,<br>
> -fred-<br>
><br>
<br>
</div></div>_______________________________________________<br>
cairo mailing list<br>
<a href="mailto:cairo@cairographics.org">cairo@cairographics.org</a><br>
<a href="http://lists.cairographics.org/mailman/listinfo/cairo" target="_blank">http://lists.cairographics.org/mailman/listinfo/cairo</a><br>
</blockquote></div><br></div>