[PATCH] D16688: [test-suite] A number of cmake configuration fixes for External/SPEC.

[Kristof Beyls via llvm-commits]

kristof.beyls added inline comments.

================
Comment at: External/SPEC/CINT2000/176.gcc/CMakeLists.txt:7-9
@@ -6,2 +6,5 @@
 endif()
+if ("${CMAKE_CXX_COMPILER_ID}" MATCHES "Clang")
+  list(APPEND CFLAGS -std=gnu89)
+endif()
 
----------------
MatzeB wrote:
> I think we can add that flag unconditionally.
Ah yes, indeed.
I tried to make sure that the cmake files allow to also run with gcc instead of clang, but -std=gnu89 is indeed also a gcc command line option with the same semantics.

================
Comment at: External/SPEC/CINT2000/252.eon/CMakeLists.txt:5-6
@@ -4,4 +4,4 @@
 endif()
-list(APPEND CXXFLAGS -fno-exceptions -Wno-deprecated -fpermissive -stdlib=libstdc++)
-list(APPEND LDFLAGS -stdlib=libstdc++ -lm)
 list(APPEND LIBS -lm)
----------------
MatzeB wrote:
> Without the -stdlib=libstdc++ the benchmark does not compile for me because it cannot find <iostream.h>. I'm surprised that worked with clang/OS X for you, maybe it is because you have an older version than me. I guess we need something like a global LIBSTDCXX_AVAILABLE variable that defaults to true if APPLE is set?
It turns out my copy of spec2000 has been modified to work around the iostream.h issue, by adding an iostream.h in the eon source code directory, which just does:
#include <iostream>
using namespace std;


I really dislike changing the source code of the benchmark, but I also don't really like being stuck to use libstdc++ and not libc++.
I think that the ideal work-around would be to add a workaround like "#include <iostream>\nusing namespace std;", but without having to change the SPEC source code. I'm not sure what the least-convoluted way would be to achieve something like that from just adapting the 252.eon/CMakeLists.txt file. I'll think a bit more about this next week, if no one else comes up with a reasonable suggestion by then.

================
Comment at: External/SPEC/CINT2000/252.eon/CMakeLists.txt:8-10
@@ -7,1 +7,5 @@
 list(APPEND LIBS -lm)
+if ("${CMAKE_CXX_COMPILER_ID}" MATCHES "Clang")
+  list(APPEND CPPFLAGS -Wno-implicit-function-declaration)
+endif()
+
----------------
MatzeB wrote:
> Why do you need to disable this specific warning? I would assume we get many other warnings which are not disabled as well.
> Also you can write it the if like this:
> ```
> if (CMAKE_CXX_COMPILER_ID MATCHES "Clang")
> ```
Strange, I thought I needed it to disable an error message, but I can't reproduce it. Let's remove this from my patch.

================
Comment at: External/SPEC/CINT2000/253.perlbmk/CMakeLists.txt:47-49
@@ -42,2 +46,5 @@
 endif()
+if ("${CMAKE_CXX_COMPILER_ID}" MATCHES "Clang")
+  list(APPEND CPPFLAGS -std=gnu89)
+endif()
 
----------------
MatzeB wrote:
> We can add that flag unconditionally.
agreed, will change that in the next version of this patch.


http://reviews.llvm.org/D16688