Opened 12 years ago

Closed 11 years ago

#1588 closed task (fixed)

[PATCH] Make 0ad compile with clang/llvm

Reported by: retrosnub Owned by:
Priority: Nice to Have Milestone: Alpha 13
Component: Build & Packages Keywords: patch clang llvm
Cc: Patch:

Description

I've made a go at trying to compile 0ad with the clang/llvm toolchain and created a minimal patch (in regard to changes to the code) to get it compile.

Tested running linux and compiled using

cd 0ad/build/workspaces
CC=clang CXX=clang++ ./update-workspaces.sh -j3
cd gcc
CC=clang CXX=clang++ make -j3

The tests were successfull and I didn't notice any problems.

It'd be nice if we could make 0ad clang-compatible as clang provides better diagnostic messages (besides profiling and other stuff).

Attachments (2)

clang_build_fix.patch (851 bytes ) - added by retrosnub 12 years ago.
20121226-223508.log.xz (26.5 KB ) - added by Julian Ospald 11 years ago.
0ad-12995_alpha12:20121226-223508.log.xz

Download all attachments as: .zip

Change History (18)

by retrosnub, 12 years ago

Attachment: clang_build_fix.patch added

comment:1 by historic_bruno, 12 years ago

Keywords: patch added
Milestone: BacklogAlpha 12

What errors do you get without these changes?

comment:2 by retrosnub, 12 years ago

The first one during the update_workspaces step when building nvtt:

…
Linking CXX shared library libnvtt.so
/usr/bin/ld: squish/libsquish.a(weightedclusterfit.cpp.o): relocation R_X86_64_32S against `_ZTVN6squish18WeightedClusterFitE' can not be used when making a shared object; recompile with -fPIC
squish/libsquish.a: could not read symbols: Bad value

And the second one during the make step (some kind of conflict with the "is_signed" symbol used in libstdc++, found this bug http://llvm.org/bugs/show_bug.cgi?id=9804 but I have no idea how the conflict is caused, maybe 0ad is using some custom libstdc headers but I haven't investigated further):

…
In file included from ../../../source/collada/PSAConvert.cpp:46:
In file included from /usr/include/c++/4.6/iterator:63:
In file included from /usr/include/c++/4.6/ostream:39:
In file included from /usr/include/c++/4.6/ios:44:
In file included from /usr/include/c++/4.6/bits/basic_ios.h:38:
In file included from /usr/include/c++/4.6/bits/locale_facets.h:2607:
/usr/include/c++/4.6/bits/locale_facets.tcc:468:57: error: expected unqualified-id
          (__negative && __gnu_cxx::__numeric_traits<_ValueT>::__is_signed)
                                                               ^
/usr/include/c++/4.6/bits/locale_facets.tcc:572:44: error: expected unqualified-id
                && __gnu_cxx::__numeric_traits<_ValueT>::__is_signed)
                                                         ^
/usr/include/c++/4.6/bits/locale_facets.tcc:895:48: error: expected unqualified-id
                    && __gnu_cxx::__numeric_traits<_ValueT>::__is_signed)
                                                             ^

Clang version:

$ clang -v
Ubuntu clang version 3.0-6ubuntu3 (tags/RELEASE_30/final) (based on LLVM 3.0)
Target: x86_64-pc-linux-gnu
Thread model: posix
Last edited 12 years ago by retrosnub (previous) (diff)

comment:3 by historic_bruno, 12 years ago

Hmm that first error seems like it could be NVTT/Squish not properly supporting or detecting Clang? There have been a lot of changes to NVTT to support clang since the last release. Unfortunately it's not as simple as applying one or two patches to test that :(

As for the second error, I have no idea what to say about it. I tested on Ubuntu 11.10 and didn't get that error with the following clang version:

clang version 2.9 (tags/RELEASE_29/final)
Target: x86_64-pc-linux-gnu
Thread model: posix

comment:4 by historic_bruno, 12 years ago

Tested with LLVM/Clang 3.1 on Ubuntu 11.10 (the binary version downloaded from llvm.org). The first (NVTT) error still occurs, but not the second, so it seems like either a bug in version 3.0 that was fixed or a problem specific to your system's combination of stdc++ and clang 3.0.

However I did find a new problem with 3.1, I get a seg fault in llvm::ScalarEvolution::getSignedRange() while compiling CCmpRangeManager.cpp. I might try again with their latest SVN version to see if that's resolved, if I get time.

comment:5 by retrosnub, 12 years ago

Using latest git for both llvm and clang I'm getting strange pragma visibility errors during the spidermonkey build:

In file included from ../jsapi.cpp:78:
../jsproxy.h:52:7: error: visibility does not match previous declaration
class JS_FRIEND_API(JSProxyHandler) {
      ^
../jstypes.h:177:29: note: expanded from macro 'JS_FRIEND_API'
#define JS_FRIEND_API(t)    JS_PUBLIC_API(t)
                            ^
../jstypes.h:167:29: note: expanded from macro 'JS_PUBLIC_API'
# define JS_PUBLIC_API(t)   JS_EXPORT_API(t)
                            ^
../jstypes.h:120:33: note: expanded from macro 'JS_EXPORT_API'
# define JS_EXPORT_API(__type)  JS_EXTERNAL_VIS __type
                                ^
../jstypes.h:101:42: note: expanded from macro 'JS_EXTERNAL_VIS'
#  define JS_EXTERNAL_VIS __attribute__((visibility ("default")))
                                         ^
../config/gcc_hidden.h:2:13: note: previous attribute is here
#pragma GCC visibility push(hidden)
            ^

found these bug reports: http://llvm.org/bugs/show_bug.cgi?id=13688 and http://llvm.org/bugs/show_bug.cgi?id=13662

I haven't been able to reduce it to a testcase and file a report yet.

comment:6 by retrosnub, 12 years ago

As a note: latest spidermonkey builds fine with clang. Could be a problem with 0ad's version of spidermonkey.

Last edited 12 years ago by retrosnub (previous) (diff)

comment:7 by historic_bruno, 11 years ago

Milestone: Alpha 12Alpha 13

comment:8 by historic_bruno, 11 years ago

Looking a bit more into the original clang issue, an fPIC option was added to NVTT for GNU C++ in revision 806. IMO the cleanest solution would be to detect Clang there and submit a patch upstream.

It appears there's not a built-in CMake variable for Clang, but I found this solution which seems to work:

Index: libraries/nvtt/src/src/nvtt/squish/CMakeLists.txt
===================================================================
--- libraries/nvtt/src/src/nvtt/squish/CMakeLists.txt	(revision 12842)
+++ libraries/nvtt/src/src/nvtt/squish/CMakeLists.txt	(working copy)
@@ -22,7 +22,11 @@
 
 ADD_LIBRARY(squish STATIC ${SQUISH_SRCS})
 
-IF(CMAKE_COMPILER_IS_GNUCXX)
+IF("${CMAKE_CXX_COMPILER}" MATCHES "clang(\\+\\+)?$")
+	SET(CMAKE_COMPILER_IS_CLANGXX 1)
+ENDIF()
+
+IF(CMAKE_COMPILER_IS_GNUCXX OR CMAKE_COMPILER_IS_CLANGXX)
 	SET_TARGET_PROPERTIES(squish PROPERTIES COMPILE_FLAGS -fPIC)
-ENDIF(CMAKE_COMPILER_IS_GNUCXX)
+ENDIF(CMAKE_COMPILER_IS_GNUCXX OR CMAKE_COMPILER_IS_CLANGXX)
 

There's also CMAKE_CXX_COMPILER_ID, but people say it shouldn't be relied upon, since it's an internal variable.

in reply to:  8 ; comment:9 by historic_bruno, 11 years ago

Replying to historic_bruno:

IMO the cleanest solution would be to detect Clang there and submit a patch upstream.

Done. The patch changed slightly to make it work with ccache+Clang.

by Julian Ospald, 11 years ago

Attachment: 20121226-223508.log.xz added

0ad-12995_alpha12:20121226-223508.log.xz

comment:10 by Julian Ospald, 11 years ago

alpha12 does not compile with clang-3.2

# clang --version clang version 3.2 (tags/RELEASE_32/final) Target: x86_64-pc-linux-gnu Thread model: posix

# cat /etc/portage/env/clang FEATURES="-ccache -buildpkg -distcc -splitdebug" USE="clang" CC=clang CXX=clang++ CFLAGS="-march=core2 -O3 -Wall" CXXFLAGS="${CFLAGS}"

Last edited 11 years ago by Julian Ospald (previous) (diff)

in reply to:  10 comment:11 by leper, 11 years ago

Replying to hasufell:

alpha12 does not compile with clang-3.2

r13031 should contain a fix.

comment:12 by ben, 11 years ago

In 13069:

Fixes NVTT build with clang, refs #1588

comment:13 by historic_bruno, 11 years ago

Keywords: review removed
Milestone: Alpha 13Backlog

in reply to:  9 comment:14 by historic_bruno, 11 years ago

Replying to historic_bruno:

Replying to historic_bruno:

IMO the cleanest solution would be to detect Clang there and submit a patch upstream.

Done. The patch changed slightly to make it work with ccache+Clang.

Note: fixed upstream by r1363.

comment:15 by historic_bruno, 11 years ago

Build reported by hasufell to work with clang-3.2 (though not testing all bundled libs). It works for me on OS X with latest Xcode command line tools / LLVM:

Apple LLVM version 4.2 (clang-425.0.27) (based on LLVM 3.2svn)

comment:16 by historic_bruno, 11 years ago

Milestone: BacklogAlpha 13
Resolution: fixed
Status: newclosed
Note: See TracTickets for help on using tickets.