[llvm-commits] [PATCH] Add MSVC multi processor compilation to build system

[Óscar Fuentes]

Erik Olofsson <Erik.Olofsson at hansoft.se>
writes:

> Adds a cmake option to compile multiple translation units in parallel
> for Microsoft compilers. Option is only added for MSVC and is enabled
> by default.
>
> Please review and commit if good.

IMHO, we must have a very good reason for enabling by default a feature
that it is disabled by default on the build tool. If the user is
expected to add /MP to his project setting in case he wants
multiprocessor builds, we shouldn't do more.

> Index: cmake/modules/HandleLLVMOptions.cmake
> ===================================================================
> --- cmake/modules/HandleLLVMOptions.cmake	(revision 126745)
> +++ cmake/modules/HandleLLVMOptions.cmake	(working copy)
> @@ -134,6 +134,13 @@
>      -w14062 # Promote "enumerator in switch of enum is not handled" to level 1 warning.
>      )
>  
> +  if (LLVM_ENABLE_MULTIPROCESSORCOMPILATION)
> +    add_llvm_definitions( /MP )
> +    message("Multi processor compilation enabled.")
> +  else (LLVM_ENABLE_MULTIPROCESSORCOMPILATION)
> +    message("Multi processor compilation disabled.")

There is no need to inform the user about both states. We must print a
message when the user changed the default value of the option.

> +  endif (LLVM_ENABLE_MULTIPROCESSORCOMPILATION)
> +
>    # Enable warnings
>    if (LLVM_ENABLE_WARNINGS)
>      add_llvm_definitions( /W4 /Wall )
> Index: CMakeLists.txt
> ===================================================================
> --- CMakeLists.txt	(revision 126745)
> +++ CMakeLists.txt	(working copy)
> @@ -133,6 +133,10 @@
>  
>  include(AddLLVMDefinitions)
>  
> +if( MSVC )
> +  option(LLVM_ENABLE_MULTIPROCESSORCOMPILATION "Enable multiprocessor compilation." ON)
> +endif()
> +

MSVC is true for all build tools that work with the MSVC compiler. AFAIK
/MP only works with MSBuild or whatever is used nowadays by the
IDE. Please use MSVC_IDE instead of MSVC.

>  option(LLVM_ENABLE_PIC "Build Position-Independent Code" ON)
>  
>  include(config-ix)

The option needs to be documented on docs/CMake.html. Maybe on the
section docs/CMake.html#msvc