Issue463

Title clang compilation on Linux
Priority feature Status resolved
Superseder Nosy List herry, jendrik, malte, rpgoldman
Assigned To malte Keywords
Optional summary

Created on 2014-09-03.16:15:56 by herry, last changed by malte.

Messages
msg3815 (view) Author: malte Date: 2014-10-13.19:20:55
Thanks for the suggestion! I've integrated this with some minor changes.
msg3814 (view) Author: herry Date: 2014-10-13.18:58:28
Thank you. I was successfully compiling FD on Linux with Clang. It looks like the 
binary generated by Clang is different with the one generated by GCC. For example, the 
size of Clang's downward-release binary is 29008795 bytes, while GCC's is 37843700 
bytes. I haven't done any performance comparison yet, but I'm interested to know the 
results.

Btw, I think there is some glitches on build_all script. I got below errors when 
invoking

$ ./build_all distclean

...
strip: 'validate': No such file
cp: cannot stat ‘validate’: No such file or directory

The following patch fixed the errors:

diff -r a2a9abe83299 src/build_all
--- a/src/build_all	Sat Oct 11 19:36:41 2014 +0200
+++ b/src/build_all	Mon Oct 13 17:56:58 2014 +0100
@@ -27,6 +27,11 @@
 
 cd VAL
 make "$@" || exit 1
-strip validate
-cp validate ../
+if [ -f validate ]; then
+	strip validate
+	cp validate ../
+fi
 cd ..
+if [ "$1" == "distclean" ]; then
+   rm -f validate
+fi
msg3811 (view) Author: malte Date: 2014-10-13.17:46:23
Yes, the main branch. The following commands work for me on Linux:

$ cd src
$ ./build_all distclean
$ ./build_all CXX=clang++ -j4

(You might want to adapt "-j4" depending on how many cores your machine has.)

Note that I got a warning, which our Makefile turned into an error, when trying
this with an older version of clang. Versions 3.3 and 3.4 worked for me; version
3.0 did not.
msg3810 (view) Author: herry Date: 2014-10-13.17:35:16
Hi Malte, Jendrik

Many thanks for having a look this issue.

It looks like FD can now be compiled using Clang. Is this available in the main 
mercurial branch? If not, where could I get this branch? I would like to try it in my 
MacOS & Linux.

Thanks,
Herry
msg3659 (view) Author: jendrik Date: 2014-10-05.20:28:17
I will check the issues on the buildbot tomorrow.

At least clang's build times are quite nice:

$ make clean
$ time CXX=clang++ make -j4
...
real	0m46.217s
user	2m35.650s
sys	0m5.977s

$ make clean
$ time CXX=g++ make -j4
...
real	1m11.075s
user	4m9.014s
sys	0m8.515s
msg3625 (view) Author: malte Date: 2014-10-04.15:35:19
Ah, no, that buildbot issue is unrelated. Jendrik, did you change something from
USE_LP=0 to USE_LP=1 somewhere on the buildbot? If yes, this needs a full
recompile (make distclean; make).
msg3624 (view) Author: malte Date: 2014-10-04.15:32:47
> But I don't expect major problems.

I was thinking to write "I don't expect problems", but thought better of it and
added "major". ;-) The buildbot complained about the change, and I think the
reason is that I didn't test this with USE_LP=1. I'll have a look.
msg3622 (view) Author: malte Date: 2014-10-04.15:15:24
Never mind: I went ahead and merged this. The changes looked sufficiently
uncontroversial to me, and it would be good to get this merged so that we can
make progress on our C++11 plans.

Of course, you can still review the code if you want -- we can back out the
changes if there's a problem. But I don't expect major problems.
msg3621 (view) Author: malte Date: 2014-10-04.15:11:10
The commits for my branch are at
https://bitbucket.org/malte/downward/commits/branch/issue463.
msg3620 (view) Author: malte Date: 2014-10-04.14:54:52
I've split off the Mac-specific things to issue484.


The two largest classes of warnings with clang on Linux are:

1) Using "struct" to define a struct, but "class" to forward-declare it.

gcc doesn't mind this, but clang gives a warning. I've fixed these in my local
issue branch.

2) Using the set<T> member template function of the Options class, which clang
warns conflicts with the set<T> standard library class template.

This is a bit of an oddity in C++98 that is fixed in C++11, and the warning
disappears when clang is run with C++11 compatibility. So I've worked towards
making the code compile with clang in C++11 mode.

This required a bunch of changes in the h^m heuristic, which used a "tuple"
typedef conflicting with the new tuple template in the standard library. I've
renamed the class to Tuple, which is more in line with our conventions anyway,
and used the opportunity to clean up h^m a bit.

With these changes and a few minor other ones, the code compiles for me using
clang++ with the options in the Makefile changed to

CXXFLAGS += -std=c++11 -Wall -Wextra -pedantic -Wno-deprecated -Werror

(i.e., using c++11 instead of c++98 and leaving the rest as-is).


My suggestion is to merge this (without the Makefile change). It won't work now,
but it will work once we switch to c++11, which will hopefully happen soon.

Jendrik, do you want to review the code?
msg3618 (view) Author: malte Date: 2014-10-04.12:54:16
OK, I've started to have a closer look. I don't think we can merge the patch
as-is because there are too many #ifdefs for clean maintenance, and there are
some performance-related changes that need to be tested (e.g., at least in gcc,
hash_set and unordered_set used to have, and likely still have, a different
memory footprint), but thanks for getting us started!

Regarding the changes in the bash scripts, I suggest we will wait for issue414
to be merged first, which will make the plan and downward scripts obsolete as
well as get rid of the (g)awk dependency.

I'd like to separate out the two aspects "clang support" and "clang *on OSX*
support" because compiler issues and operating system issues are largely
separate. So my first aim is to make the code compile cleanly using clang on
Linux. The good news here is that the current code in the default branch already
compiles with clang on Linux with

CXX=clang++ make

if we disable the "warnings as errors" option. So it's just a matter of compiler
warnings, not actual compiler errors. Still, it would be good to get to the
bottom of the warnings and address them. I'll work on it.
msg3412 (view) Author: herry Date: 2014-09-04.22:01:38
You're welcome. Please buzz me if you're going to address this issue.
msg3410 (view) Author: malte Date: 2014-09-03.19:55:09
Thanks! I've unassigned the 1.0 keyword, as we're only using this internally to
categorize certain issues.

It would be nice to add clang support, but it will take us a while to clear the
backlog of other issues that are currently under way and get back to this one.
(At the moment, I think the patch is too invasive to be merged as is, so I think
some more work is needed. We also need to think about a way of making it easier
to maintain clang compatibility in the future.)

I hope that we will be able to make progress on this issue in October. If not,
please feel free to remind us to get back to it!
msg3409 (view) Author: herry Date: 2014-09-03.16:21:38
Note that the changes have been moved to branch issue 463:

https://bitbucket.org/hherry/fast-downward/branch/issue463
msg3408 (view) Author: herry Date: 2014-09-03.16:15:56
To be able compiling FastDownward in MacOS X, we have to either install MacPort or 
Homebrew, which might not convinience for some users. Thus, it would be better if the 
current codes are updated so that clang (default OSX compiler) can compile them. The 
preprocess components is fully compatible with clang, but the search engine is not.

Recently, I have been able to compile the codes purely using clang with some 
modifications, which can be obtained in:

https://bitbucket.org/hherry/fast-downward/branch/clang
History
Date User Action Args
2014-10-13 19:20:55maltesetmessages: + msg3815
2014-10-13 18:58:28herrysetmessages: + msg3814
2014-10-13 17:46:23maltesetmessages: + msg3811
2014-10-13 17:35:16herrysetmessages: + msg3810
2014-10-05 20:28:17jendriksetmessages: + msg3659
2014-10-05 19:15:42rpgoldmansetnosy: + rpgoldman
2014-10-04 15:35:20maltesetstatus: in-progress -> resolved
messages: + msg3625
2014-10-04 15:32:47maltesetstatus: resolved -> in-progress
messages: + msg3624
2014-10-04 15:15:24maltesetstatus: in-progress -> resolved
messages: + msg3622
2014-10-04 15:11:10maltesetmessages: + msg3621
2014-10-04 14:54:52maltesetstatus: reviewing -> in-progress
messages: + msg3620
title: Clang compilation on OSX -> clang compilation on Linux
2014-10-04 12:54:16maltesetassignedto: malte
messages: + msg3618
2014-09-04 22:01:38herrysetmessages: + msg3412
2014-09-03 19:55:09maltesetmessages: + msg3410
keyword: - 1.0
2014-09-03 17:33:05jendriksetnosy: + malte, jendrik
2014-09-03 16:21:38herrysetmessages: + msg3409
2014-09-03 16:15:56herrycreate