[Pixman] Pixman not building on MacOS X 10.11

[Andrea Canciani]

On Sun, Oct 11, 2015 at 5:30 AM, Siarhei Siamashka <
siarhei.siamashka at gmail.com> wrote:

> On Sun, 11 Oct 2015 04:53:08 +0300
> Siarhei Siamashka <siarhei.siamashka at gmail.com> wrote:
>
> > On Sat, 10 Oct 2015 16:03:53 -0700
> > Jeremy Huddleston Sequoia <jeremyhu at freedesktop.org> wrote:
> >
> > > > On Oct 10, 2015, at 13:48, Andrea Canciani <ranma42 at gmail.com>
> wrote:
> > > > The attached hack gets the code to compile on modern clang, but I
> > > > believe first of all we should improve the configure.ac detection
> code
> > > > so that pixman can actually build both on old and on new clang
> > > > versions (possibly with mmx disabled, if the asm constraints we need
> > > > are not implemented).
> >
> > This workaround looks reasonable to me. We should probably just drop
> > the whole "ifdef __OPTIMIZE__" part in
> >
> http://cgit.freedesktop.org/pixman/tree/pixman/pixman-mmx.c?id=pixman-0.32.8#n92
> >
> > I don't quite like the fact that this way of returning results from
> > a macro is a GNU C specific extension. But as you said, the configure
> > test can be updated to better match the code and also check if the
> > compiler supports this particular construct.
> >
> > Could you please submit the final variant of your patch in a
> > "git format-patch" format with a commit message and your
> > Signed-off-by tag?
>
> After looking at this issue a bit more, I realized that we are
> about to add a second layer of workarounds on top of the existing
> old workarounds :-)
>

The attached patch should fix the issue with only minor changes.
It keeps the workarounds :( but somewhat it simplifies them :)
I followed your suggestion of checking&using block expressions.
Given that the _mm_shuffle_pi16() function is always used in a "return"
statement, if needed we could avoid the usage of block expressions by
defining a macro "_return_mm_shuffle_pi16()" (which would return the result
of the operation instead of making it available as an expression) both for
the xmmintrin branch and for the hand-coded one.

The original problem is that certain compilers (just GCC?) did not
> support some intrinsics when compiling MMX code (_mm_movemask_pi8,
> _mm_mulhi_pu16, _mm_shuffle_pi16) and we got the following code:
>
> http://cgit.freedesktop.org/pixman/tree/pixman/pixman-mmx.c?id=pixman-0.32.8#n66
>
> In fact, these instructions were not available as part of the original
> MMX, but only got introduced later with AMD Extended 3DNow! and Intel
> SSE1. This is mentioned in the commit messages:
>
> http://cgit.freedesktop.org/pixman/commit/?id=84221f4c1687b8ea14e9cbdc78b2ba7258e62c9e
>
> http://cgit.freedesktop.org/pixman/commit/?id=14208344964f341a7b4a704b05cf4804c23792e9
>
> These extra instructions are unofficially known as MMX2. But GCC does
> not have a separate option for "-mmmx2". Instead the GCC manual says
> that these intrinsics are available when either "-msse" or a
> combination of "-m3dnow -march=athlon" is used:
>
> https://gcc.gnu.org/onlinedocs/gcc-5.2.0/gcc/x86-Built-in-Functions.html#x86-Built-in-Functions
>
>
> Now I wonder if the comment "We have to compile with -msse to use
> xmmintrin.h" is still valid. I tried to tweak the following ifdef to
> use the part of code, which includes <xmmintrin.h> and the it compiled
> fine for me with CFLAGS="-O2 -m32" using recent versions of GCC and
> Clang:
>
> http://cgit.freedesktop.org/pixman/tree/pixman/pixman-mmx.c?id=pixman-0.32.8#n63
>
> I believe that this might be somehow related to the new __ALL_ISA__
> define, which had been mentioned in 2013:
>     https://gcc.gnu.org/ml/gcc-patches/2013-04/txts5M0c0uU9y.txt
>
> So what about just dropping this ugly stuff and adding a configure
> check, which would verify if the MMX code can include <xmmintrin.h>?
>

I would love getting rid of the workarounds, but I'm somewhat worried about
the possibility of regressions.
If you believe is a valid option, we might definitely try to pursue it.

What is the best way forward?

Andrea

And if this configure check fails (because of using an old compiler),
> then simply disable MMX support when building pixman.
>
> --
> Best regards,
> Siarhei Siamashka
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/pixman/attachments/20151011/23b31107/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-mmx-Improve-detection-of-support-for-K-constraint.patch
Type: application/octet-stream
Size: 2621 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/pixman/attachments/20151011/23b31107/attachment.obj>