[flang] Fix release build flags.

[DavidTruby]

Summary

This patch removes the custom CMAKE_RELEASE_CXX_FLAGS variable.
This variable being set was having the effect of removing other important
Release flags, notably -DNDEBUG.

This patch may need to be accompanied by fixes for the macOS issues that
the removed comment mentions; I don't have a mac to test this on though so
hopefully a reviewer can help with that.

[tskeith]

This builds for me with clang 9 on macos and I'm seeing -O3 -DNDEBUG on the clang++ command line.

[sscalpone]

@tskeith Did you or perhaps @pmccormick report the original problem on mac?

[tskeith]

In D80794#2063880, @sscalpone wrote:

@tskeith Did you or perhaps @pmccormick report the original problem on mac?

I don't think I reported it.

[pmccormick]

In D80794#2063920, @tskeith wrote:

In D80794#2063880, @sscalpone wrote:

@tskeith Did you or perhaps @pmccormick report the original problem on mac?

I don't think I reported it.

I had it listed as a TODO item in the CMake file but did not file an issue...

[sscalpone]

In D80794#2064046, @pmccormick wrote:

I had it listed as a TODO item in the CMake file but did not file an issue...

I didn't mean to make anyone feel guilty :) I'm just curious is this is a mac-only problem (which Tim reports is fixed in his environment) or if we observed it elsewhere.

[awarzynski]

LGTM

Builds fine on my Mac (Mac OS Mojave) with Apple Clang 10.0.1. For reference:

cmake -G Ninja -DLLVM_CCACHE_BUILD=On -DLLVM_APPEND_VC_REV=Off -DLLVM_TARGETS_TO_BUILD=X86 -DLLVM_ENABLE_PROJECTS="clang;flang;mlir" -DCMAKE_BUILD_TYPE=Release ../../llvm/
ninja

[Meinersbur]

The CMAKE_XYZ_FLAGS are supposed to be in control by the user, not by the project. Modifying them in the CMakeLists.txt is an anti-pattern. It has global effects, that is -DDEBUGF18 will be added to all LLVM projects in debug builds, not just flang. Did you consider using add_compile_definitions or target_compile_definitions instead?

[DavidTruby]

In D80794#2074790, @Meinersbur wrote:

The CMAKE_XYZ_FLAGS are supposed to be in control by the user, not by the project. Modifying them in the CMakeLists.txt is an anti-pattern. It has global effects, that is -DDEBUGF18 will be added to all LLVM projects in debug builds, not just flang. Did you consider using add_compile_definitions or target_compile_definitions instead?

I can look into that as well; I didn't write the original code I just spotted that we were not defining NDEBUG in Release builds and found a fix for that.

[isuruf]

It has global effects, that is -DDEBUGF18 will be added to all LLVM projects in debug builds, not just flang

Are you sure about this? CMake variables are scoped and would apply only to flang.

flang/CMakeLists.txt

set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wno-unused-command-line-argument -Wstring-conversion \
           -Wcovered-switch-default")
   endif()  # Clang.
 
   check_cxx_compiler_flag("-Werror -Wnested-anon-types" CXX_SUPPORTS_NO_NESTED_ANON_TYPES_FLAG)
   if (CXX_SUPPORTS_NO_NESTED_ANON_TYPES_FLAG)
     set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wno-nested-anon-types")
   endif()
 
-  # Add to or adjust build type flags.
-  #
-  # TODO: This needs some extra thought.  CMake's default for release builds
-  # is -O3, which can cause build failures on certain platforms (and compilers)
-  # with the current code base -- some templated functions are inlined and don't
-  # become available at link time when using -O3 (with Clang under MacOS/darwin).
-  # If we reset CMake's default flags we also clobber any user provided settings;
-  # make it difficult to customize a build in this regard...   The setup below
-  # has this side effect but enables successful builds across multiple platforms
-  # in release mode...
+  # Add to build type flags.
   set(CMAKE_CXX_FLAGS_DEBUG "${CMAKE_CXX_FLAGS_DEBUG} -DDEBUGF18")
-  set(CMAKE_CXX_FLAGS_MINSIZEREL "${CMAKE_CXX_FLAGS_MINSIZEREL} -DCHECK=\"(void)\"")  # do we need -O2 here?
-  set(CMAKE_CXX_FLAGS_RELEASE "-O2")
+  set(CMAKE_CXX_FLAGS_MINSIZEREL "${CMAKE_CXX_FLAGS_MINSIZEREL} -DCHECK=\"(void)\"")
 
   # Building shared libraries is bad for performance with GCC by default
   # due to the need to preserve the right to override external entry points
   if (BUILD_SHARED_LIBS AND NOT (CMAKE_CXX_COMPILER_ID MATCHES "Clang"))
    set(CMAKE_CXX_FLAGS_RELEASE "${CMAKE_CXX_FLAGS_RELEASE} -fno-semantic-interposition")
   endif()
 
 endif()