Bug 151441 - [jhbuild] Fix pixman build with clang
Summary: [jhbuild] Fix pixman build with clang
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: Other
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Csaba Osztrogonác
URL:
Keywords:
Depends on:
Blocks: 145121
  Show dependency treegraph
 
Reported: 2015-11-19 04:10 PST by Csaba Osztrogonác
Modified: 2015-11-27 05:56 PST (History)
4 users (show)

See Also:


Attachments
Patch (1.90 KB, patch)
2015-11-19 04:34 PST, Csaba Osztrogonác
cgarcia: review+
Details | Formatted Diff | Diff
Patch (2.43 KB, patch)
2015-11-27 05:23 PST, Csaba Osztrogonác
no flags Details | Formatted Diff | Diff
Patch (2.44 KB, patch)
2015-11-27 05:38 PST, Csaba Osztrogonác
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Csaba Osztrogonác 2015-11-19 04:10:05 PST
$ clang --version
Ubuntu clang version 3.6.0-2ubuntu1 (tags/RELEASE_360/final) (based on LLVM 3.6.0)
Target: x86_64-pc-linux-gnu
Thread model: posix

build error:
-------------
pixman-mmx.c:100:20: error: constraint 'K' expects an integer constant expression
 : "y" (__A), "K" (__N)
                   ^~~

related bug reports:
- https://github.com/Homebrew/homebrew/issues/41056
- http://lists.freedesktop.org/archives/pixman/2015-October/004080.html

Let's disable MMX optimizations to make clang happy.
Comment 1 Csaba Osztrogonác 2015-11-19 04:34:32 PST
Created attachment 265859 [details]
Patch
Comment 2 Carlos Garcia Campos 2015-11-19 05:05:26 PST
If this is a problem only for clang, would it be possible to detect it somehow from jhbuildrc and add the autogen command line argument there?
Comment 3 Csaba Osztrogonác 2015-11-19 05:23:42 PST
(In reply to comment #2)
> If this is a problem only for clang, would it be possible to detect it
> somehow from jhbuildrc and add the autogen command line argument there?

I don't know if it is possible, but maybe it is easier to patch
configure.ac to disable mmx. It has some check for this, but
it seems it doesn't work properly. I'll check it later.
Comment 4 Michael Catanzaro 2015-11-19 05:35:26 PST
(In reply to comment #3)
> (In reply to comment #2)
> > If this is a problem only for clang, would it be possible to detect it
> > somehow from jhbuildrc and add the autogen command line argument there?
> 
> I don't know if it is possible, but maybe it is easier to patch
> configure.ac to disable mmx. It has some check for this, but
> it seems it doesn't work properly. I'll check it later.

Much better IMO to edit jhbuildrc as you've done than to try patching and regenerating configure. I would just leave a FIXME comment with a link to this bug so that it can be reverted in the future.

But really, I wonder why you're using clang for the dependencies. I understand using it for WebKit itself, but for an entire jhbuild moduleset of dependencies outside our control, I bet something will always be broken.
Comment 5 Csaba Osztrogonác 2015-11-19 05:40:45 PST
(In reply to comment #4)
> (In reply to comment #3)
> > (In reply to comment #2)
> > > If this is a problem only for clang, would it be possible to detect it
> > > somehow from jhbuildrc and add the autogen command line argument there?
> > 
> > I don't know if it is possible, but maybe it is easier to patch
> > configure.ac to disable mmx. It has some check for this, but
> > it seems it doesn't work properly. I'll check it later.
> 
> Much better IMO to edit jhbuildrc as you've done than to try patching and
> regenerating configure. I would just leave a FIXME comment with a link to
> this bug so that it can be reverted in the future.

It can be much better if I know how to do it. But now I have no idea
if it is possible to detect the compiler by jhbuild. And I don't have
any time to investigate jhbuild internals in the near future.

> But really, I wonder why you're using clang for the dependencies. I
> understand using it for WebKit itself, but for an entire jhbuild moduleset
> of dependencies outside our control, I bet something will always be broken.

Because I don't want to switch compiler settings between jhbuild and 
build-webkit. The final goal is to make everything work with GCC ang clang too.
Comment 6 Michael Catanzaro 2015-11-19 08:52:02 PST
(In reply to comment #5)
> It can be much better if I know how to do it. But now I have no idea
> if it is possible to detect the compiler by jhbuild. And I don't have
> any time to investigate jhbuild internals in the near future.

You can do it in the .jhbuildrc by checking for CC in the environment, but that's just as yucky as a patch. You can't do it from the moduleset file.

Anyway, I would simply disable the thing unconditionally, just as you've done, so it's the same regardless of which compiler is used. I like your current patch as-is; I would just also add FIXME comments above those changes.
Comment 7 Carlos Garcia Campos 2015-11-19 10:10:49 PST
Comment on attachment 265859 [details]
Patch

Ok, let's follow the easiest approach, then.
Comment 8 Csaba Osztrogonác 2015-11-27 05:23:18 PST
Created attachment 266199 [details]
Patch

Patch for landing with additional FIXME comment.
Comment 9 Csaba Osztrogonác 2015-11-27 05:38:49 PST
Created attachment 266200 [details]
Patch

Patch for landing with additional FIXME comment.
Comment 10 Csaba Osztrogonác 2015-11-27 05:56:38 PST
Committed r192780: <http://trac.webkit.org/changeset/192780>