Have target/host triples "arch-vendor-os" as in autotools #326

Merged
merged 3 commits into from May 9, 2016

Conversation

Projects
None yet
4 participants
@tammoippen
Contributor

tammoippen commented Apr 29, 2016

Many internal functions, e.g. memory_thisjob, require the target triples "arch-vendor-os" as they were supplied with autotools. The file GetTriple.cmake adds functions for retrieving these. The rest of this PR sets the appropriate defines in config.h.in.

I propose @jakobj and @jougs as reviewers.

Have target/host triples as in autotools
Many internal functions, e.g. memory_thisjob, require the target triples "arch-vendor-os" as they were supplied with autotools. The file GetTriple.cmake adds functions for retrieving these. The rest of this commit sets the appropriate defines in config.h.in
+
+ # Get os.
+ if ( "${CMAKE_HOST_SYSTEM_NAME}" STREQUAL "Windows" )
+ set( os "win32" )

This comment has been minimized.

@jougs

jougs Apr 30, 2016

Contributor

Wouldn't just "win" be more correct on current versions of that OS?

@jougs

jougs Apr 30, 2016

Contributor

Wouldn't just "win" be more correct on current versions of that OS?

This comment has been minimized.

@tammoippen

tammoippen May 9, 2016

Contributor

I am not so sure how relevant this actually is, given that NEST is not officially supported on Windows ("we do not tests this regularly [building on Windows] and do not support NEST under Cygwin at present"), but the lines are only relevant, if the host system name is Windows, then this will default to win32. In other cases, the CMAKE_HOST_SYSTEM_NAME will just be cast to lower case. Maybe on a 64bit Windows this will default to win64 - I have not tested this - or on MinGW this will default to mingw32 (as the autoconf counterpart).

@tammoippen

tammoippen May 9, 2016

Contributor

I am not so sure how relevant this actually is, given that NEST is not officially supported on Windows ("we do not tests this regularly [building on Windows] and do not support NEST under Cygwin at present"), but the lines are only relevant, if the host system name is Windows, then this will default to win32. In other cases, the CMAKE_HOST_SYSTEM_NAME will just be cast to lower case. Maybe on a 64bit Windows this will default to win64 - I have not tested this - or on MinGW this will default to mingw32 (as the autoconf counterpart).

+
+ # Get os.
+ if ( "${CMAKE_SYSTEM_NAME}" STREQUAL "Windows" )
+ set( os "win32" )

This comment has been minimized.

@jougs

jougs Apr 30, 2016

Contributor

See above

@jougs

jougs Apr 30, 2016

Contributor

See above

@@ -115,6 +115,10 @@ find_program( SED NAMES sed gsed )
################## CPack, checks, ... in external modules ##################
################################################################################
+include( GetTriple )

This comment has been minimized.

@jougs

jougs Apr 30, 2016

Contributor

I don't like the name GetTriple for this file. Can you make it more specific, please?MaybeDetermineArchitecture` or something like this.

@jougs

jougs Apr 30, 2016

Contributor

I don't like the name GetTriple for this file. Can you make it more specific, please?MaybeDetermineArchitecture` or something like this.

This comment has been minimized.

@tammoippen

tammoippen May 9, 2016

Contributor

In autotools they are called TypeTriplets. Other cmake implementations call this GetTriplet.cmake, or Triplet.cmake[1] [2]. As this is specifically for the autotools triplets, I think the name triplet should be contained.

@tammoippen

tammoippen May 9, 2016

Contributor

In autotools they are called TypeTriplets. Other cmake implementations call this GetTriplet.cmake, or Triplet.cmake[1] [2]. As this is specifically for the autotools triplets, I think the name triplet should be contained.

@jougs

This comment has been minimized.

Show comment
Hide comment
@jougs

jougs Apr 30, 2016

Contributor

This seems like a useful addition to me. See my comments above. 👍 once they are addressed.

Contributor

jougs commented Apr 30, 2016

This seems like a useful addition to me. See my comments above. 👍 once they are addressed.

@jakobj

This comment has been minimized.

Show comment
Hide comment
@jakobj

jakobj May 1, 2016

Contributor

I can confirm that this works nicely with my machine (Debian 8), and our cluster (CentOS 7.1). I can't really comment on the code, so 👍 from me. :)

Contributor

jakobj commented May 1, 2016

I can confirm that this works nicely with my machine (Debian 8), and our cluster (CentOS 7.1). I can't really comment on the code, so 👍 from me. :)

cmake/GetTriple.cmake
+ if ( "${CMAKE_SYSTEM_NAME}" MATCHES "^Darwin.*")
+ set( vendor "apple" )
+ else ()
+ set( vendor "pc" )

This comment has been minimized.

@heplesser

heplesser May 1, 2016

Contributor

Would this set vendor to pc also for BlueGene and K?

@heplesser

heplesser May 1, 2016

Contributor

Would this set vendor to pc also for BlueGene and K?

@jakobj

This comment has been minimized.

Show comment
Hide comment
@jakobj

jakobj May 2, 2016

Contributor

@heplesser on juqueen this creates the following triples:

Host triple: ppc64-pc-linux
Target triple: -pc-bluegeneq_base

target triple looks a bit broken to me?!

Contributor

jakobj commented May 2, 2016

@heplesser on juqueen this creates the following triples:

Host triple: ppc64-pc-linux
Target triple: -pc-bluegeneq_base

target triple looks a bit broken to me?!

@jakobj

This comment has been minimized.

Show comment
Hide comment
@jakobj

jakobj May 2, 2016

Contributor

and on K:

Host triple: x86_64-pc-linux
Target triple: s64fx-pc-linux

this looks fine to me.

Contributor

jakobj commented May 2, 2016

and on K:

Host triple: x86_64-pc-linux
Target triple: s64fx-pc-linux

this looks fine to me.

@heplesser

This comment has been minimized.

Show comment
Hide comment
@heplesser

heplesser May 3, 2016

Contributor

@jakobj That target triple for BG/Q looks a bit strange since the first component is empty and the last part quite long.

But all these triples label BG/Q and K as pc, which doesn't cause any harm, I believe, but is a bit bit misleading ...

Contributor

heplesser commented May 3, 2016

@jakobj That target triple for BG/Q looks a bit strange since the first component is empty and the last part quite long.

But all these triples label BG/Q and K as pc, which doesn't cause any harm, I believe, but is a bit bit misleading ...

@tammoippen

This comment has been minimized.

Show comment
Hide comment
@tammoippen

tammoippen May 9, 2016

Contributor

For NEST the relevant triple is the target triple, which is only different from the host triplet in the case of cross compiling. The target triplet for BG/Q is now:

-- Target triple: ppc64-ibm-linux

For the K computer it is:

-- Target triple: s64fx-fujitsu-linux

Contributor

tammoippen commented May 9, 2016

For NEST the relevant triple is the target triple, which is only different from the host triplet in the case of cross compiling. The target triplet for BG/Q is now:

-- Target triple: ppc64-ibm-linux

For the K computer it is:

-- Target triple: s64fx-fujitsu-linux

@heplesser heplesser merged commit 7d421b7 into nest:master May 9, 2016

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@tammoippen tammoippen deleted the tammoippen:hostos branch May 18, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment