[Openmp-dev] Initial Intel Cmake Build System Patch

[Peyton, Jonathan L]

> After the NetBSD stuff - I may contribute a Solaris patch for fun...
It is always fun to port things! :)

> The Windows part doesn't seem to give any consideration for other compiler (Fortran/ASM/C/C++)?
> if(${WINDOWS})
> # I guess it's ok for me to do a follow-up patch for other compiler support? I could see the patch being intrusive or making the code ugly though. :-/ # My first thought is remove the if/elseif logic entirely and move all these > variable to an external toolchain file runtime/cmake/icc.cmake It would have a lot of variables like
> WIN_FC_FLAGS_64 = "-nologo -Qdiag-disable:177,5082 -GS -DynamicBase"
>
> if(${DYNAMIC_LIBRARY})|
> ||list(APPEND ||WIN_FC_FLAGS ||-Zi)|
> endif()
> if(${RELEASE})|
> ...
> |
A long time ago, we used to support Visual Studio on Windows.  Currently, only icl for the C/C++ compiler and ml/ml64 for the assembler on Windows.
Go ahead and add other compiler support for Windows.  That would be great.  I don't know about moving icc specific stuff to a different file.  I was real relaxed about wrapping the Windows specific flags under 
If(${ICC}).  It might be strange to have LinkerFlags.cmake, CFlags.cmake, etc. and also an icc.cmake (which would include Windows only icc?).

>###
>Nits
>
>I think there's a "cmake" way to get the OS version?
>+function(set_mac_os_new return_mac_os_new)
>
>Would this do what you want?
>http://www.cmake.org/Wiki/CMake_Useful_Variables
>CMAKE_SYSTEM
I am not sure there is a useful way to get the OSX version in a purely cmake way.  The CMAKE_SYSTEM variable doesn't correspond to the OS X version, only the Darwin Kernel version.
I would love to get rid of this though.

>Not everyone (very few) systems will have cmake this new. cmake is typically very easy to bootstrap, but may be more friendly if backed off a bit. (Doesn't impact me, but esp for TIMESTAMP. Custom command or some >default could be provided..)
>+# - this function alone causes the need for CMake v2.8.11 (TIMESTAMP)
I was thinking hard about this before and I think I will add some code which will call the date command on unix systems.  The problem was Windows does not have a clean way to get the TIMESTAMP.  The current way Windows does is using some Perl, but I refuse to add any more Perl.  I think you are right to back off a bit.

>Why not set SSE2? (Anyone who cries fowl.. are you really going to be running OMP on a processor THAT old?)
I think before changing this, it needs to be shown that real performance improvements could be gained.

-----Original Message-----
From: "C. Bergström" [mailto:cbergstrom at pathscale.com] 
Sent: Saturday, June 7, 2014 1:08 AM
To: Peyton, Jonathan L
Cc: openmp-dev at dcs-maillist2.engr.illinois.edu
Subject: Re: [Openmp-dev] Initial Intel Cmake Build System Patch

On 06/ 7/14 06:02 AM, Peyton, Jonathan L wrote:
>
> Hello All,
>
> Since everyone wants to see it, this patch is the initial cmake build 
> system we have been developing. Some quick notes:
>
> 1) The top-level CMakeLists.txt you all have been working on is moved 
> to CMakeLists.txt.old (the src/CMakeLists.txt file is untouched)
>
> 2) There is a Build_With_CMake.txt tutorial that shows the most 
> important features and how to build libiomp5
>
> 3) As a special note, the current build.pl system does five 
> "micro-tests" which test certain features of the newly built 
> libiomp5.so library (basic linking/compilation, library dependencies,
> etc.) If you want these to be off in the new CMake system just specify 
> it when calling cmake with the flag -Dtests=off
>
> Please keep in mind this is the initial system and that it can be 
> trimmed back incrementally to what you need. Right now, it builds 
> nearly identical libraries as build.pl. On a related note, I attempted 
> ( as suggested by David ) to port libiomp5 to NetBSD to see what walls 
> I would hit. I hit some J , but eventually got it working (not 
> included in this initial patch). So I am going to follow up this 
> initial patch with further improvements (to the build system) to make 
> re-targeting in general even easier. Some of these improvements will 
> also help the build.pl system as well.
>
> Just try it. See what you hate and what you like. Leave feedback. I am 
> here on weekdays.
>
After the NetBSD stuff - I may contribute a Solaris patch for fun...

I realize I'm a corner case, but some things I'll hit

The Windows part doesn't seem to give any consideration for other compiler (Fortran/ASM/C/C++)?
if(${WINDOWS})
# I guess it's ok for me to do a follow-up patch for other compiler support? I could see the patch being intrusive or making the code ugly though. :-/ # My first thought is remove the if/elseif logic entirely and move all these variable to an external toolchain file runtime/cmake/icc.cmake It would have a lot of variables like
WIN_FC_FLAGS_64 = "-nologo -Qdiag-disable:177,5082 -GS -DynamicBase"

if(${DYNAMIC_LIBRARY})|
||list(APPEND ||WIN_FC_FLAGS ||-Zi)|
endif()
if(${RELEASE})|
...
|

I can probably do this, but your feedback/help is appreciated.


###
Nits

I think there's a "cmake" way to get the OS version?
+function(set_mac_os_new return_mac_os_new)

Would this do what you want?
http://www.cmake.org/Wiki/CMake_Useful_Variables
CMAKE_SYSTEM

Not everyone (very few) systems will have cmake this new. cmake is typically very easy to bootstrap, but may be more friendly if backed off a bit. (Doesn't impact me, but esp for TIMESTAMP. Custom command or some default could be provided..)
+# - this function alone causes the need for CMake v2.8.11 (TIMESTAMP)

Why not set SSE2? (Anyone who cries fowl.. are you really going to be running OMP on a processor THAT old?)

(nit)Correct?
---------
+ else()
+ append_c_source_file("z_Linux_util.c")
+ append_c_source_file("kmp_gsupport.c")
-------
I'll wait for your feedback on the above before reviewing more..