[flang][cmake] Enable the new driver by default

[awarzynski]

Summary

With this patch:

• FLANG_BUILD_NEW_DRIVER is set to On by default (i.e. the new driver is enabled)
• Clang is automatically added to the list of dependencies.

By setting FLANG_BUILD_NEW_DRIVER to Off, the new driver will be
disabled and Clang will be removed from the list of dependencies.

I tried implementing this logic in Flang's top CMake script, but that's
too late for updating LLVM_ENABLE_PROJECTS (which is used to
enable/disable Clang).

[Meinersbur]

llvm/CMakeLists.txt

78

Could you move the option into the if ("flang" IN_LIST LLVM_ENABLE_PROJECTS) condition so at least it does not appear unless flang is enabled?

84

[nit] Space between ( and FLANG_BUILD_NEW_DRIVER. While not done consistently done in the file, the patch itself could itself be consistent with itself.

86

CACHE variables are user-controlled and should not be modified. Actually, this creates a new non-cached local variables that shadows the cached variable.

From the CMake manual:

Normally projects should avoid using normal and cache variables of thesame name, as this interaction can be hard to follow.

[awarzynski]

llvm/CMakeLists.txt

86

Thanks for pointing this out. It sounds like we shouldn't be touching LLVM_ENABLE_PROJECTS at all.

We should also remove list(APPEND LLVM_ENABLE_PROJECTS "mlir"), but I'd rather send a separate patch for this.

[awarzynski]

Address PR comments

Thank you for taking a look @Meinersbur. It sounds that instead of updating LLVM_ENABLE_PROJECTS, it will be better to generate an error when Clang is missing. What do you think?

[jdoerfert]

Not a cmake expert, if @Meinersbur is happy with this we can go ahead. There was no objection on the call we discussed this nor do I see one here.

[MehdiChinoune]

In D101842#2744348, @awarzynski wrote:

Address PR comments

Thank you for taking a look @Meinersbur. It sounds that instead of updating LLVM_ENABLE_PROJECTS, it will be better to generate an error when Clang is missing. What do you think?

You can use set(LLVM_ENABLE_PROJECTS "${LLVM_ENABLE_PROJECTS}" "clang" CACHE STRING "list of projects to build" FORCE)
to force-update it.

[Meinersbur]

In D101842#2744957, @MehdiChinoune wrote:

You can use set(LLVM_ENABLE_PROJECTS "${LLVM_ENABLE_PROJECTS}" "clang" CACHE STRING "list of projects to build" FORCE)
to force-update it.

This duplicates the option line with the setting description. Generally, I think user-controlled configuration parameters should stay user-controlled. Controlling the defaults for LLVM_TOOL_*_BUILD might be a better option.

I agree this would be matter of another patch to fix.

[awarzynski]

@MehdiChinoune , thank you for your suggestion. I agree with with @Meinersbur and prefer to merge this as is. Just waiting for the buildbot to be back online.

flang/CMakeLists.txt

 cmake_minimum_required(VERSION 3.13.4)
 
 set(CMAKE_BUILD_WITH_INSTALL_NAME_DIR ON)
 
-option(FLANG_BUILD_NEW_DRIVER "Build the flang compiler driver" OFF)
-
 # Flang requires C++17.
 set(CMAKE_CXX_STANDARD 17)
 set(CMAKE_CXX_STANDARD_REQUIRED TRUE)
 set(CMAKE_CXX_EXTENSIONS OFF)
 
 set(FLANG_SOURCE_DIR ${CMAKE_CURRENT_SOURCE_DIR})
 
 if (CMAKE_SOURCE_DIR STREQUAL CMAKE_BINARY_DIR AND NOT MSVC_IDE)

flang/README.md

 
 The instructions to build LLVM can be found at
 https://llvm.org/docs/GettingStarted.html. If you are building flang as part
 of LLVM, follow those instructions and add flang to `LLVM_ENABLE_PROJECTS`.
 
 We highly recommend using the same compiler to compile both llvm and flang.
 
 The flang CMakeList.txt file uses
-the variable `LLVM_DIR` to find the installed LLVM components
-and
-the variable `MLIR_DIR` to find the installed MLIR components.
+* `LLVM_DIR` to find the installed LLVM components
+* `MLIR_DIR` to find the installed MLIR components
+* `CLANG_DIR` to find the installed Clang components
 
-To get the correct LLVM and MLIR libraries included in your flang build,
-define LLVM_DIR and MLIR_DIR on the cmake command line.
+To get the correct LLVM, MLIR and Clang libraries included in your flang build,
+define `LLVM_DIR`, `MLIR_DIR` and `CLANG_DIR` on the cmake command line.
 ```
 LLVM=<LLVM_BUILD_DIR>/lib/cmake/llvm \
 MLIR=<LLVM_BUILD_DIR>/lib/cmake/mlir \
-cmake -DLLVM_DIR=$LLVM -DMLIR_DIR=$MLIR ...
+CLANG=<LLVM_BUILD_DIR>/lib/cmake/clang \
+cmake -DLLVM_DIR=$LLVM -DMLIR_DIR=$MLIR -DCLANG_DIR=$CLANG ...
 ```
 where `LLVM_BUILD_DIR` is
 the top-level directory where LLVM was built.
 
 ### Building flang with GCC
 
 By default,
 cmake will search for g++ on your PATH.

 add
 `-DCMAKE_BUILD_TYPE=Release`
 to the cmake command.
 Release builds execute quickly.
 
 ### Build Flang out of tree
 ```
 cd ~/flang/build
-cmake -DLLVM_DIR=$LLVM -DMLIR_DIR=$MLIR ~/flang/src
+cmake -DLLVM_DIR=$LLVM -DMLIR_DIR=$MLIR -DCLANG_DIR=$CLANG ~/flang/src
 make
 ```
 
-### Build The New Flang Driver
-The new Flang driver, `flang-new`, is currently under active development and
-should be considered as an experimental feature. For this reason it is disabled
-by default. This will change once the new driver replaces the _throwaway_
-driver, `flang`.
-
-In order to build the new driver, add `-DFLANG_BUILD_NEW_DRIVER=ON` to your
-CMake invocation line. Additionally, when building out-of-tree, use `CLANG_DIR`
-(similarly to `LLVM_DIR` and `MLIR_DIR`) to find the installed Clang
-components.
-
-**Note:** `CLANG_DIR` is only required when building the new Flang driver,
-which currently depends on Clang.
+### Disable The New Flang Driver
+The new Flang compiler driver, `flang-new`, is implemented in terms of
+`clangDriver` and hence it introduces a dependency on Clang. This dependency is
+otherwise not required. If you do not require the new driver, you can disable
+it by adding `-DFLANG_BUILD_NEW_DRIVER=OFF` to your CMake invocation. With the
+new driver disabled, you no longer need to add `clang` to
+`LLVM_ENABLE_PROJECTS` (or to specify `CLANG_DIR` when building out-of-tree).
 
 # How to Run Tests
 
 Flang supports 2 different categories of tests
 1. Regression tests (https://www.llvm.org/docs/TestingGuide.html#regression-tests)
 2. Unit tests (https://www.llvm.org/docs/TestingGuide.html#unit-tests)
 
 ## For out of tree builds

llvm/CMakeLists.txt

 set(LLVM_EXTRA_PROJECTS "flang")
 # List of all known projects in the mono repo
 set(LLVM_KNOWN_PROJECTS "${LLVM_ALL_PROJECTS};${LLVM_EXTRA_PROJECTS}")
 set(LLVM_ENABLE_PROJECTS "" CACHE STRING
   "Semicolon-separated list of projects to build (${LLVM_KNOWN_PROJECTS}), or \"all\".")
 if( LLVM_ENABLE_PROJECTS STREQUAL "all" )
   set( LLVM_ENABLE_PROJECTS ${LLVM_ALL_PROJECTS})
 endif()
-if ("flang" IN_LIST LLVM_ENABLE_PROJECTS AND NOT "mlir" IN_LIST LLVM_ENABLE_PROJECTS)
+
+if ("flang" IN_LIST LLVM_ENABLE_PROJECTS)
+  if (NOT "mlir" IN_LIST LLVM_ENABLE_PROJECTS)
     message(STATUS "Enabling MLIR as a dependency to flang")
     list(APPEND LLVM_ENABLE_PROJECTS "mlir")
   endif()
 
+  option(FLANG_BUILD_NEW_DRIVER "Build the flang compiler driver" ON)
+  if (FLANG_BUILD_NEW_DRIVER AND NOT "clang" IN_LIST LLVM_ENABLE_PROJECTS)
+    message(FATAL_ERROR "Clang is not enabled, but it's required by the new Flang driver")
+  endif()
+endif()
+
 # LLVM_ENABLE_PROJECTS_USED is `ON` if the user has ever used the
 # `LLVM_ENABLE_PROJECTS` CMake cache variable.  This exists for
 # several reasons:
 #
 # * As an indicator that the `LLVM_ENABLE_PROJECTS` list is now the single
 # source of truth for which projects to build. This means we will ignore user
 # supplied `LLVM_TOOL_<project>_BUILD` CMake cache variables and overwrite
 # them.