[Openmp-dev] Initial Intel Cmake Build System Patch

[Peyton, Jonathan L]

C. Bergström,

It's fine if you want to change the flag-grabbing layout.  Although, I feel having icc.cmake, gcc.cmake, clang.cmake, common.cmake, etc. would be just as hard to maintain.  i.e., If some flag is only valid for a subset of compilers then all those *.cmake files will have to be modified.  Each file will also have its own if(OS_TYPE) sections.  If you really want to support every possible compiler, you are just going to have to strip away as many of the compiler-specific flags as you can.

>>> 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.
>Anecdotal evidence, but SSE2 is probably one of the most helpful x86 extensions from my experience. Why not enable it?
I'm not opposed.  I don't have much to hang my hat on concerning this issue.  Maybe someone else can chime in on this?  It would be very easy to change!

Johnny 

-----Original Message-----
From: "C. Bergström" [mailto:cbergstrom at pathscale.com] 
Sent: Monday, June 9, 2014 12:40 PM
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/ 9/14 11:52 PM, Peyton, Jonathan L wrote:
>> 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?).
You misunderstand - I mean 1 configure file per toolchain. No LinkerFlags.cmake.. etc.. For ICC you'd create 1 file with all the things you'd need. icc.cmake

Sorry, the more I look at this the more I'm apposed to your original design. It will lead to spaghetti mess as time goes on an increasingly become harder to maintain. /* Sure for only ICC or another compiler it's fine, but would fly with multiple gcc versions, clang, icc, MSVC, pathscale, UH compiler.. etc */
>
>> ###
>> 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.
Custom command is easy enough to do this. There may even be .cmake in the wild which helps. Maybe poking the cmake dev list is a good idea here. Alternatively a feature test..
>
>> 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.
Anecdotal evidence, but SSE2 is probably one of the most helpful x86 extensions from my experience. Why not enable it?