[Pixman] Pixman not building on MacOS X 10.11

[Siarhei Siamashka]

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:
> > 
> > It looks like the latest XCode cannot build pixman:
> > 
> > $ clang --version
> > Apple LLVM version 7.0.0 (clang-700.0.72)
> > Target: x86_64-apple-darwin14.5.0
> > Thread model: posix
> > 
> > pixman-mmx.c:100:20: error: constraint 'K' expects an integer constant expression
> >         : "y" (__A), "K" (__N)
> >                           ^~~
> > Reported here:
> > https://github.com/Homebrew/homebrew/issues/41056

Hello Andrea.

Thanks for posting all the important details to the mailing list.
We already had a report about this clang related issue:
    http://lists.freedesktop.org/archives/pixman/2015-September/004036.html

Don't know why, but previously I had an impression that "the homebrew
built version of pixman" was just a slang for saying "compiled pixman
from sources myself". It's nice to know that "Homebrew" is the name of
a project on github. Now everything makes a bit more sense :-)

Meanwhile, I have upgraded my clang from 3.5.0 to 3.7.0:
    $ clang -v
    clang version 3.7.0 (tags/RELEASE_370/final)
    Target: x86_64-pc-linux-gnu
    Thread model: posix
    Selected GCC installation: /usr/lib/gcc/x86_64-pc-linux-gnu/4.8.5

And now I can also reproduce this problem. Looks like very old clang
versions did not support this asm constraint (that was when we first
encountered this problem), then clang got support for it for a while.
But newer versions of clang have decided to reject this code again.
 
> > It looks like the test performed in configure.ac does not match the
> > code with sufficient accuracy.
> > In particular, the constant is hardcoded in the configure.ac test,
> > while in the code it is only available after inlining.

Yes, ideally the configure tests should try doing exactly the same
things as the actual code.

> > 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?

> This is not a bug in clang.  It isn't a case of "the asm constraints
> we need are not implemented" it is a case of the code not being
> valid.  The error printed by clang is correct.  See this post for more
> details:
> https://code.google.com/p/nativeclient/issues/detail?id=4201#c5

Hello Jeremy.

Well, it's difficult to argue about the code being valid or not
because we are relying on the compiler-specific extensions here.
The __always_inline__ attribute implementation is not a part
of the official C standard. GCC just implements it because this
is a useful feature:

   https://gcc.gnu.org/onlinedocs/gcc-5.2.0/gcc/Function-Attributes.html#Function-Attributes
   "always_inline
      Generally, functions are not inlined unless optimization is
      specified. For functions declared inline, this attribute inlines
      the function independent of any restrictions that otherwise apply
      to inlining. Failure to inline such a function is diagnosed as an
      error. Note that if such a function is called indirectly the
      compiler may or may not inline it depending on optimization level
      and a failure to inline an indirect call may or may not be
      diagnosed."

And then Clang is trying its best to be GCC compatible. In a nutshell,
we have the following situation:

    static always_inline void foo(int x)
    {
        /* Clang can't see that 'x' is always a compile time
           constant here, but GCC can */
    }

    void bar(void)
    {
        foo(1);
        foo(23);
        foo(45);
    }

Because the compilers are not perfect and sometimes fail our
expectations, we are forced to resort to adding workarounds
occasionally. GCC also had a problem with inlining functions
some time ago:
    http://cgit.freedesktop.org/pixman/commit/?id=9df645dfb04b5a790faabe1e9a84fc37287d91b0

> Or the description of the K constraint here:
> https://gcc.gnu.org/onlinedocs/gcc/Simple-Constraints.html#Simple-Constraints

Yes, that's exactly what we need here. Because the PSHUFW instruction
expects an integer constant as the third operand.

> I think a simple  (albeit not quite elegant) fix would be to just use X instead of K.

The "X" constraint is not a correct fix because it's a wrong constraint.
If the operand happens to be anything other than integer 8-bit constant
known at compile time, then we will just upset the assembler.

-- 
Best regards,
Siarhei Siamashka