RESOLVED FIXED 151441
[jhbuild] Fix pixman build with clang 
https://bugs.webkit.org/show_bug.cgi?id=151441
Summary [jhbuild] Fix pixman build with clang
Csaba Osztrogonác
Reported 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.
Attachments
Patch (1.90 KB, patch)
2015-11-19 04:34 PST, Csaba Osztrogonác 		cgarcia: review+
DetailsFormatted DiffDiff
Patch (2.43 KB, patch)
2015-11-27 05:23 PST, Csaba Osztrogonác 		no flags
DetailsFormatted DiffDiff
Patch (2.44 KB, patch)
2015-11-27 05:38 PST, Csaba Osztrogonác 		no flags
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
Csaba Osztrogonác
Comment 1 2015-11-19 04:34:32 PST
Created attachment 265859 [details] Patch
Carlos Garcia Campos
Comment 2 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?
Csaba Osztrogonác
Comment 3 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.
Michael Catanzaro
Comment 4 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.
Csaba Osztrogonác
Comment 5 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.
Michael Catanzaro
Comment 6 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.
Carlos Garcia Campos
Comment 7 2015-11-19 10:10:49 PST
Comment on attachment 265859 [details] Patch Ok, let's follow the easiest approach, then.
Csaba Osztrogonác
Comment 8 2015-11-27 05:23:18 PST
Created attachment 266199 [details] Patch Patch for landing with additional FIXME comment.
Csaba Osztrogonác
Comment 9 2015-11-27 05:38:49 PST
Created attachment 266200 [details] Patch Patch for landing with additional FIXME comment.
Csaba Osztrogonác
Comment 10 2015-11-27 05:56:38 PST
Committed r192780: <http://trac.webkit.org/changeset/192780>
Note You need to log in before you can comment on or make changes to this bug.