Set -fp-model precise when using the ICPC+Linux

[owainkenwayucl]

As per discussion in https://github.com/mrc-ide/covid-sim/issues/358 this adds the flag -fp-model precise to the C++ compiler flags when compiling with the Intel compiler on Linux which aids passing regression tests on that platform.

[weshinsley]

I think this is a good idea - and possibly should be applied to the Windows Intel compiler as well. Hard to test without having an Intel C++ compiler for linux...

[owainkenwayucl]

@weshinsley Similarly, the reason I restricted this patch to Linux is that I can't test it on Windows (and the Intel compiler options have a different format on Windows so I really would want to test it).

I have tested this on our Linux systems at it works as intended, i.e. on the Intel compilers it sets -fp-model precise and on the GNU + clang ones it doesn't.

[owainkenwayucl]

It does seem to be trying to do something similar on lines 19-22 for Windows but in my reading of the documentation I'm not sure add_compile_options("fp:strict")actually does anything to targets declared before it is set on some versions of CMake, e.g. for 3.13.x:

Adds options to the compiler command line for targets in the current directory and below that are added after this command is invoked. See documentation of the directory and target COMPILE_OPTIONS properties.

But then for 3.17.x it says:

Adds options to the COMPILE_OPTIONS directory property. These options are used when compiling targets from the current directory and below.

so I'm guessing the behaviour has changed.

In the root CMakeLists.txt for the project the minimum version is 3.8, so I guess the Windows rules may or may not be applied correctly depending on what version people are using.

[weshinsley]

I'll have to ask about this a bit more as far as cmakelists goes...

What I did find out though, is that when compiling with Intel from within the Visual Studio integration, VS's default (/fp:precise) is sent on the Intel compiler commandline by default. (Which is nice)

[matt-gretton-dann]

There is target_compile_options() which I think should be also used for the Visual Studio FP precise statement lower down?

[owainkenwayucl]

@weshinsley what should I do to progress this getting merged?

[weshinsley]

Sorry @owainkenwayucl I'd forgotten about this - I'm not very familiar with cmake and I'd put it to one side to read up on.

I think what @matt-gretton-dann means is, put the if (intel) add_compile_options bit just after line 23 inside the elseif(UNIX),

Let's not worry about Intel/Windows on this PR - I'll hook that up and test in a bit...

[owainkenwayucl]

Ah right - also my brain was not working as I misunderstood target_compile_options as being somehow what was already being done but that is of course add_compile_options. CMake be like that.

[owainkenwayucl]

So I tried with target_compile_options on the Unix side and it works so it may be worth adapting the Windows part appropriately:

--- CMakeLists.txt.orig 2020-07-20 11:47:38.000000000 +0100
+++ CMakeLists.txt      2020-07-20 11:47:46.000000000 +0100
@@ -21,6 +21,9 @@
   endif()
 elseif(UNIX)
   target_compile_definitions(CovidSim PUBLIC UNIX)
+  if(CMAKE_CXX_COMPILER_ID STREQUAL "Intel")
+    target_compile_options(CovidSim PUBLIC "SHELL:-fp-model precise")
+  endif()
 else()
   message(FATAL_ERROR "Unknown operating system type: ${CMAKE_SYSTEM_NAME}")
 endif()

[owainkenwayucl]

(Just to confirm - I've tested that this does the right thing both with Intel Compilers and GCC on Linux)

[weshinsley]

Yep, and all tests pass. Ok, let's get this one in, and I'll back to the Intel command-line options for Windows