This is an archive of the discontinued LLVM Phabricator instance.

Differential D14561

Add CMakeLists for External/{Nurbs|Povray|skidmarks10|HMMER}
ClosedPublic

Authored by MatzeB on Nov 10 2015, 6:49 PM.

Details

Reviewers
rengolin
jmolloy
beanz
Commits
rOLDT254834: Add CMakeLists for External/{Nurbs|Povray|skidmarks10|HMMER}
rL254834: Add CMakeLists for External/{Nurbs|Povray|skidmarks10|HMMER}
Summary

Adds a way to configure the location of test-suite externals and adds CMakeLists for Nurbs, Povray, skidmarks10 and HMMER.

Diff Detail

Event Timeline

MatzeB updated this revision to Diff 39882.Nov 10 2015, 6:49 PM
MatzeB retitled this revision from to Add CMakeLists for External/{Nurbs|Povray|skidmarks10|HMMER}.
MatzeB updated this object.
MatzeB added reviewers: jmolloy, rengolin, beanz.
MatzeB added a subscriber: llvm-commits.
jmolloy requested changes to this revision.Nov 11 2015, 8:48 AM
jmolloy edited edge metadata.

Hi Matthias,

Thanks for pushing this forward. I've got some comments below.

Cheers,

James

CMakeLists.txt
59

Don't we want something like this instead?:

if(CMAKE_SIZEOF_VOID_P EQUAL 8)
  set(TMP_LP64 TRUE)
else()
  set(TMP_LP64 FALSE)
endif()
set(ARCH_LP64 ${TMP_LP64} CACHE BOOL "Does the target architecture has 64-bit pointers?")

I'll defer to Chris B on this

71

I'd prefer if we had a "LLVM_" prefix on all variables we expect to be set by the user. Can we stick to that, like in the other projects' builds?

External/CMakeLists.txt
2

I think this relies on a CMake module from LLVM, not from test-suite, so it'll fail unless we're in /projects.

Just doing "add_subdirectory()" several times isn't much harder.

This revision now requires changes to proceed.Nov 11 2015, 8:48 AM
beanz added inline comments.Nov 11 2015, 9:29 AM
CMakeLists.txt
59

Why does this need to be cached at all?

I generally prefer not using cached variables unless there is a really good reason because CMake has no cache invalidation.

71

Rather than LLVM, I would suggest prefixing with TEST_SUITE.

I think each project should have its own prefix. Also, with r252747 variables that start with TEST_SUITE will automatically be passed from the top-level CMake invocation into the sub-project configuration.

External/CMakeLists.txt
2

llvm_add_subdirectories actually comes from test-suite, so this is fine.

MatzeB added inline comments.Nov 11 2015, 9:42 AM
CMakeLists.txt
59

My goal here was not so much the caching but more about giving the user control over these settings. Though I guess the proper design would be to only give a variable to the user to override the auto detection instead of saving the result of the auto detection in the cache.

Anyway I agree that in this specific instance it's probably best to not make it cached at all.

71

TEST_SUITE_ prefix sounds good.

External/CMakeLists.txt
2

I can change the layout to this to give more of a "one-item in each line" feeling which should be good for subdirectories.

llvm_add_subdirectories(
  HMMER
  Nurbs
  Povray
  skidmarks10
)
MatzeB updated this revision to Diff 40202.Nov 13 2015, 8:33 PM
MatzeB edited edge metadata.

Adapted to review feedback.

MatzeB added a child revision: D14681: cmake/lit: Add SPEC2000.Nov 13 2015, 9:18 PM
Closed by commit rL254834: Add CMakeLists for External/{Nurbs|Povray|skidmarks10|HMMER} (authored by matze). · Explain WhyDec 4 2015, 8:33 PM
This revision was automatically updated to reflect the committed changes.

Revision Contents

PathSize
8 lines
External/
6 lines
HMMER/
25 lines
Nurbs/
20 lines
Povray/
14 lines
skidmarks10/
21 lines

Diff 40202

CMakeLists.txt

Show First 20 Lines • Show All 48 Lines • ▼ Show 20 Linesif(NOT DEFINED ENDIAN)
include(TestBigEndian) include(TestBigEndian)
test_big_endian(IS_BIGENDIAN) test_big_endian(IS_BIGENDIAN)
if(IS_BIGENDIAN) if(IS_BIGENDIAN)
set(ENDIAN "big") set(ENDIAN "big")
else() else()
set(ENDIAN "little") set(ENDIAN "little")
endif() endif()
endif() endif()
if(CMAKE_SIZEOF_VOID_P EQUAL 8)
set(ARCH_LP64 TRUE)
endif()
jmolloyUnsubmitted
Not Done
ReplyInline Actions

Don't we want something like this instead?:

if(CMAKE_SIZEOF_VOID_P EQUAL 8)
  set(TMP_LP64 TRUE)
else()
  set(TMP_LP64 FALSE)
endif()
set(ARCH_LP64 ${TMP_LP64} CACHE BOOL "Does the target architecture has 64-bit pointers?")

I'll defer to Chris B on this

jmolloy: Don't we want something like this instead?: if(CMAKE_SIZEOF_VOID_P EQUAL 8) set(TMP_LP64…
beanzUnsubmitted
Not Done
ReplyInline Actions

Why does this need to be cached at all?

I generally prefer not using cached variables unless there is a really good reason because CMake has no cache invalidation.

beanz: Why does this need to be cached at all? I generally prefer not using cached variables unless…
MatzeBAuthorUnsubmitted
Not Done
ReplyInline Actions

My goal here was not so much the caching but more about giving the user control over these settings. Though I guess the proper design would be to only give a variable to the user to override the auto detection instead of saving the result of the auto detection in the cache.

Anyway I agree that in this specific instance it's probably best to not make it cached at all.

MatzeB: My goal here was not so much the caching but more about giving the user control over these…
set(TEST_SUITE_EXTERNALS_DIR "" CACHE PATH "Directory of llvm-test-suite external sources")
add_subdirectory(tools) add_subdirectory(tools)
# Now that the tools have been created, use tools/timeit to time all other compilations. # Now that the tools have been created, use tools/timeit to time all other compilations.
set(CMAKE_C_COMPILE_OBJECT "${CMAKE_BINARY_DIR}/tools/timeit --summary <OBJECT>.time ${CMAKE_C_COMPILE_OBJECT}") set(CMAKE_C_COMPILE_OBJECT "${CMAKE_BINARY_DIR}/tools/timeit --summary <OBJECT>.time ${CMAKE_C_COMPILE_OBJECT}")
set(CMAKE_CXX_COMPILE_OBJECT "${CMAKE_BINARY_DIR}/tools/timeit --summary <OBJECT>.time ${CMAKE_CXX_COMPILE_OBJECT}") set(CMAKE_CXX_COMPILE_OBJECT "${CMAKE_BINARY_DIR}/tools/timeit --summary <OBJECT>.time ${CMAKE_CXX_COMPILE_OBJECT}")
add_subdirectory(SingleSource) add_subdirectory(SingleSource)
add_subdirectory(MultiSource) add_subdirectory(MultiSource)
if(TEST_SUITE_EXTERNALS_DIR)
jmolloyUnsubmitted
Not Done
ReplyInline Actions

I'd prefer if we had a "LLVM_" prefix on all variables we expect to be set by the user. Can we stick to that, like in the other projects' builds?

jmolloy: I'd prefer if we had a "LLVM_" prefix on all variables we expect to be set by the user. Can we…
beanzUnsubmitted
Not Done
ReplyInline Actions

Rather than LLVM, I would suggest prefixing with TEST_SUITE.

I think each project should have its own prefix. Also, with r252747 variables that start with TEST_SUITE will automatically be passed from the top-level CMake invocation into the sub-project configuration.

beanz: Rather than LLVM, I would suggest prefixing with TEST_SUITE. I think each project should have…
MatzeBAuthorUnsubmitted
Not Done
ReplyInline Actions

TEST_SUITE_ prefix sounds good.

MatzeB: TEST_SUITE_ prefix sounds good.
add_subdirectory(External)
endif()
# Produce lit.site.cfg # Produce lit.site.cfg
configure_file("${CMAKE_SOURCE_DIR}/lit.site.cfg.in" "${CMAKE_BINARY_DIR}/lit.site.cfg") configure_file("${CMAKE_SOURCE_DIR}/lit.site.cfg.in" "${CMAKE_BINARY_DIR}/lit.site.cfg")

External/CMakeLists.txt

  • This file was added.
llvm_add_subdirectories(
HMMER
jmolloyUnsubmitted
Not Done
ReplyInline Actions

I think this relies on a CMake module from LLVM, not from test-suite, so it'll fail unless we're in /projects.

Just doing "add_subdirectory()" several times isn't much harder.

jmolloy: I think this relies on a CMake module from LLVM, not from test-suite, so it'll fail unless…
beanzUnsubmitted
Not Done
ReplyInline Actions

llvm_add_subdirectories actually comes from test-suite, so this is fine.

beanz: llvm_add_subdirectories actually comes from test-suite, so this is fine.
MatzeBAuthorUnsubmitted
Not Done
ReplyInline Actions

I can change the layout to this to give more of a "one-item in each line" feeling which should be good for subdirectories.

llvm_add_subdirectories(
  HMMER
  Nurbs
  Povray
  skidmarks10
)
MatzeB: I can change the layout to this to give more of a "one-item in each line" feeling which should…
Nurbs
Povray
skidmarks10
)

External/HMMER/CMakeLists.txt

  • This file was added.
# Autogenerated from External/HMMER/Makefile
set(DIRS)
set(PARALLEL_DIRS)
set(TEST_SUITE_HMMER_ROOT "${TEST_SUITE_EXTERNALS_DIR}/hmmer" CACHE PATH "Directory containing hmmer sourcecode")
set(LEVEL ../..)
set(PROG hmmcalibrate)
FILE(GLOB Source ${TEST_SUITE_HMMER_ROOT}/*.c)
set(CPPFLAGS -DSSE2)
list(APPEND LIBS -lpthread)
list(APPEND LDFLAGS -lpthread)
if("${ARCH}" STREQUAL "PowerPC")
list(APPEND TARGET_FLAGS -maltivec)
list(APPEND LCCFLAGS -maltivec)
endif()
if(DEFINED LARGE_PROBLEM_SIZE)
set(RUN_OPTIONS --fixed 400 --cpu 1 --num 200000 --seed 1158818515 ${TEST_SUITE_HMMER_ROOT}/globin.hmm)
else()
set(RUN_OPTIONS --fixed 400 --cpu 1 --num 80000 --seed 1158818515 ${TEST_SUITE_HMMER_ROOT}/globin.hmm)
endif()
llvm_multisource()
llvm_add_subdirectories(${DIRS} ${PARALLEL_DIRS})

External/Nurbs/CMakeLists.txt

  • This file was added.
# Autogenerated from External/Nurbs/Makefile
set(DIRS)
set(PARALLEL_DIRS)
set(TEST_SUITE_NURBS_ROOT "${TEST_SUITE_EXTERNALS_DIR}/nurbs" CACHE PATH "Directory containing nurbs sourcecode")
set(LEVEL ../..)
set(PROG nurbs)
file(GLOB Source ${TEST_SUITE_NURBS_ROOT}/*.cpp)
set(CPPFLAGS )
set(LDFLAGS -lstdc++)
list(APPEND LIBS -lstdc++)
if(ARCH_LP64)
list(APPEND CPPFLAGS -DDWORD_IS_ULL)
endif()
set(RUN_OPTIONS /k all timed /t 500 /vsteps 192 /usteps 192 /vcp 20 /ucp 20)
llvm_multisource()
llvm_add_subdirectories(${DIRS} ${PARALLEL_DIRS})

External/Povray/CMakeLists.txt

  • This file was added.
set(PROG povray)
set(TEST_SUITE_POVRAY_ROOT "${TEST_SUITE_EXTERNALS_DIR}/povray31" CACHE PATH "Directory containing povray 3.1 sourcecode")
file(GLOB Source ${TEST_SUITE_POVRAY_ROOT}/source/*.c ${TEST_SUITE_POVRAY_ROOT}/source/libpng/*.c)
list(REMOVE_ITEM Source ${TEST_SUITE_POVRAY_ROOT}/source/libpng/ansi2knr.c ${TEST_SUITE_POVRAY_ROOT}/source/libpng/pngtest.c ${TEST_SUITE_POVRAY_ROOT}/source/libpng/example.c)
list(APPEND CPPFLAGS -I${TEST_SUITE_POVRAY_ROOT}/source -I${TEST_SUITE_POVRAY_ROOT}/source/unix -I${TEST_SUITE_POVRAY_ROOT}/source/libpng -DPREFIX='"${CMAKE_CURRENT_BINARY_DIR}"' -DSYSCONFDIR='"${CMAKE_CURRENT_BINARY_DIR}"' -UHAVE_LIBVGA)
list(APPEND LIBS -lz -lm)
list(APPEND LDFLAGS -lz -lm)
set(RUN_OPTIONS -I${TEST_SUITE_POVRAY_ROOT}/scenes/advanced/chess2.pov -L${TEST_SUITE_POVRAY_ROOT}/include -GA$<.junk -O-)
llvm_multisource()

External/skidmarks10/CMakeLists.txt

  • This file was added.
# Autogenerated from External/skidmarks10/Makefile
set(DIRS)
set(PARALLEL_DIRS)
set(TEST_SUITE_SKIDMARKS10_ROOT "${TEST_SUITE_EXTERNALS_DIR}/skidmarks10" CACHE PATH "Directory containing skidmarks 10 sourcecode")
set(LEVEL ../..)
set(PROG skidmarks)
set(DISABLE_JIT 1)
file(GLOB Source ${TEST_SUITE_SKIDMARKS10_ROOT}/source/*.c)
set(RUN_OPTIONS 0 ${TEST_SUITE_SKIDMARKS10_ROOT}/golden)
list(APPEND CPPFLAGS -I${TEST_SUITE_SKIDMARKS10_ROOT}/source)
string(FIND "${ARCH}" "x86" TMP1)
if(NOT "" STREQUAL "${TMP1}")
list(APPEND CPPFLAGS -msse3)
endif()
llvm_multisource()
llvm_add_subdirectories(${DIRS} ${PARALLEL_DIRS})