[flang][windows] Add option to link against specific MSVC CRT #70833
Conversation
llvmbot
added
clang
|
@llvm/pr-subscribers-clang @llvm/pr-subscribers-flang-driver Author: David Truby (DavidTruby) ChangesCurrently flang's runtime libraries are only built for the specific CRT Full diff: https://github.com/llvm/llvm-project/pull/70833.diff 19 Files Affected:
diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td
index c8b730e0f7ecd84..66d4794714c9529 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -2842,7 +2842,7 @@ def fms_compatibility_version
"version number to report in _MSC_VER (0 = don't define it "
"(default))">;
def fms_runtime_lib_EQ : Joined<["-"], "fms-runtime-lib=">, Group<f_Group>,
- Flags<[]>, Visibility<[ClangOption, CLOption]>,
+ Flags<[]>, Visibility<[ClangOption, CLOption, FlangOption]>,
Values<"static,static_dbg,dll,dll_dbg">,
HelpText<"Select Windows run-time library">,
DocBrief<[{
diff --git a/clang/lib/Driver/ToolChains/CommonArgs.cpp b/clang/lib/Driver/ToolChains/CommonArgs.cpp
index ad012d3d0d4b46f..1cac9a179eb960f 100644
--- a/clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ b/clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -976,12 +976,46 @@ bool tools::addOpenMPRuntime(ArgStringList &CmdArgs, const ToolChain &TC,
return true;
}
-void tools::addFortranRuntimeLibs(const ToolChain &TC,
+void tools::addFortranRuntimeLibs(const ToolChain &TC, const ArgList &Args,
llvm::opt::ArgStringList &CmdArgs) {
if (TC.getTriple().isKnownWindowsMSVCEnvironment()) {
- CmdArgs.push_back("Fortran_main.lib");
- CmdArgs.push_back("FortranRuntime.lib");
- CmdArgs.push_back("FortranDecimal.lib");
+ CmdArgs.push_back(Args.MakeArgString(
+ "/DEFAULTLIB:" + TC.getCompilerRTBasename(Args, "builtins")));
+ unsigned RTOptionID = options::OPT__SLASH_MT;
+ if (auto *rtl = Args.getLastArg(options::OPT_fms_runtime_lib_EQ)) {
+ RTOptionID = llvm::StringSwitch<unsigned>(rtl->getValue())
+ .Case("static", options::OPT__SLASH_MT)
+ .Case("static_dbg", options::OPT__SLASH_MTd)
+ .Case("dll", options::OPT__SLASH_MD)
+ .Case("dll_dbg", options::OPT__SLASH_MDd)
+ .Default(options::OPT__SLASH_MT);
+ }
+ switch(RTOptionID) {
+ case options::OPT__SLASH_MT:
+ CmdArgs.push_back("/DEFAULTLIB:libcmt");
+ CmdArgs.push_back("Fortran_main.static.lib");
+ CmdArgs.push_back("FortranRuntime.static.lib");
+ CmdArgs.push_back("FortranDecimal.static.lib");
+ break;
+ case options::OPT__SLASH_MTd:
+ CmdArgs.push_back("/DEFAULTLIB:libcmtd");
+ CmdArgs.push_back("Fortran_main.static_debug.lib");
+ CmdArgs.push_back("FortranRuntime.static_debug.lib");
+ CmdArgs.push_back("FortranDecimal.static_debug.lib");
+ break;
+ case options::OPT__SLASH_MD:
+ CmdArgs.push_back("/DEFAULTLIB:msvcrt");
+ CmdArgs.push_back("Fortran_main.dynamic.lib");
+ CmdArgs.push_back("FortranRuntime.dynamic.lib");
+ CmdArgs.push_back("FortranDecimal.dynamic.lib");
+ break;
+ case options::OPT__SLASH_MDd:
+ CmdArgs.push_back("/DEFAULTLIB:msvcrtd");
+ CmdArgs.push_back("Fortran_main.dynamic_debug.lib");
+ CmdArgs.push_back("FortranRuntime.dynamic_debug.lib");
+ CmdArgs.push_back("FortranDecimal.dynamic_debug.lib");
+ break;
+ }
} else {
CmdArgs.push_back("-lFortran_main");
CmdArgs.push_back("-lFortranRuntime");
diff --git a/clang/lib/Driver/ToolChains/CommonArgs.h b/clang/lib/Driver/ToolChains/CommonArgs.h
index f364c9793c9be62..0a0951c5386e601 100644
--- a/clang/lib/Driver/ToolChains/CommonArgs.h
+++ b/clang/lib/Driver/ToolChains/CommonArgs.h
@@ -116,7 +116,7 @@ bool addOpenMPRuntime(llvm::opt::ArgStringList &CmdArgs, const ToolChain &TC,
bool IsOffloadingHost = false, bool GompNeedsRT = false);
/// Adds Fortran runtime libraries to \p CmdArgs.
-void addFortranRuntimeLibs(const ToolChain &TC,
+void addFortranRuntimeLibs(const ToolChain &TC, const llvm::opt::ArgList &Args,
llvm::opt::ArgStringList &CmdArgs);
/// Adds the path for the Fortran runtime libraries to \p CmdArgs.
diff --git a/clang/lib/Driver/ToolChains/Darwin.cpp b/clang/lib/Driver/ToolChains/Darwin.cpp
index f28e08d81bf29b4..7481f6cab9a968d 100644
--- a/clang/lib/Driver/ToolChains/Darwin.cpp
+++ b/clang/lib/Driver/ToolChains/Darwin.cpp
@@ -678,7 +678,7 @@ void darwin::Linker::ConstructJob(Compilation &C, const JobAction &JA,
// to generate executables.
if (getToolChain().getDriver().IsFlangMode()) {
addFortranRuntimeLibraryPath(getToolChain(), Args, CmdArgs);
- addFortranRuntimeLibs(getToolChain(), CmdArgs);
+ addFortranRuntimeLibs(getToolChain(), Args, CmdArgs);
}
if (!Args.hasArg(options::OPT_nostdlib, options::OPT_nodefaultlibs))
diff --git a/clang/lib/Driver/ToolChains/DragonFly.cpp b/clang/lib/Driver/ToolChains/DragonFly.cpp
index ed7f751adc0efaf..19131f45dd1ec2c 100644
--- a/clang/lib/Driver/ToolChains/DragonFly.cpp
+++ b/clang/lib/Driver/ToolChains/DragonFly.cpp
@@ -150,7 +150,7 @@ void dragonfly::Linker::ConstructJob(Compilation &C, const JobAction &JA,
// AddRunTimeLibs).
if (D.IsFlangMode()) {
addFortranRuntimeLibraryPath(ToolChain, Args, CmdArgs);
- addFortranRuntimeLibs(ToolChain, CmdArgs);
+ addFortranRuntimeLibs(ToolChain, Args, CmdArgs);
CmdArgs.push_back("-lm");
}
diff --git a/clang/lib/Driver/ToolChains/FreeBSD.cpp b/clang/lib/Driver/ToolChains/FreeBSD.cpp
index e7dcccc9e9fc4c8..e285e07e23514a2 100644
--- a/clang/lib/Driver/ToolChains/FreeBSD.cpp
+++ b/clang/lib/Driver/ToolChains/FreeBSD.cpp
@@ -297,7 +297,7 @@ void freebsd::Linker::ConstructJob(Compilation &C, const JobAction &JA,
// AddRunTimeLibs).
if (D.IsFlangMode()) {
addFortranRuntimeLibraryPath(ToolChain, Args, CmdArgs);
- addFortranRuntimeLibs(ToolChain, CmdArgs);
+ addFortranRuntimeLibs(ToolChain, Args, CmdArgs);
if (Profiling)
CmdArgs.push_back("-lm_p");
else
diff --git a/clang/lib/Driver/ToolChains/Gnu.cpp b/clang/lib/Driver/ToolChains/Gnu.cpp
index 5237951f84cce03..e38f002b04b6519 100644
--- a/clang/lib/Driver/ToolChains/Gnu.cpp
+++ b/clang/lib/Driver/ToolChains/Gnu.cpp
@@ -576,7 +576,7 @@ void tools::gnutools::Linker::ConstructJob(Compilation &C, const JobAction &JA,
// AddRunTimeLibs).
if (D.IsFlangMode()) {
addFortranRuntimeLibraryPath(ToolChain, Args, CmdArgs);
- addFortranRuntimeLibs(ToolChain, CmdArgs);
+ addFortranRuntimeLibs(ToolChain, Args, CmdArgs);
CmdArgs.push_back("-lm");
}
diff --git a/clang/lib/Driver/ToolChains/Haiku.cpp b/clang/lib/Driver/ToolChains/Haiku.cpp
index b940150788f65c7..7a41e4a99b84b39 100644
--- a/clang/lib/Driver/ToolChains/Haiku.cpp
+++ b/clang/lib/Driver/ToolChains/Haiku.cpp
@@ -101,7 +101,7 @@ void haiku::Linker::ConstructJob(Compilation &C, const JobAction &JA,
// AddRunTimeLibs).
if (D.IsFlangMode()) {
addFortranRuntimeLibraryPath(ToolChain, Args, CmdArgs);
- addFortranRuntimeLibs(ToolChain, CmdArgs);
+ addFortranRuntimeLibs(ToolChain, Args, CmdArgs);
}
CmdArgs.push_back("-lgcc");
diff --git a/clang/lib/Driver/ToolChains/MSVC.cpp b/clang/lib/Driver/ToolChains/MSVC.cpp
index 4966d102c51f1ac..8a4a174c90ea855 100644
--- a/clang/lib/Driver/ToolChains/MSVC.cpp
+++ b/clang/lib/Driver/ToolChains/MSVC.cpp
@@ -131,7 +131,7 @@ void visualstudio::Linker::ConstructJob(Compilation &C, const JobAction &JA,
if (C.getDriver().IsFlangMode()) {
addFortranRuntimeLibraryPath(TC, Args, CmdArgs);
- addFortranRuntimeLibs(TC, CmdArgs);
+ addFortranRuntimeLibs(TC, Args, CmdArgs);
// Inform the MSVC linker that we're generating a console application, i.e.
// one with `main` as the "user-defined" entry point. The `main` function is
diff --git a/clang/lib/Driver/ToolChains/MinGW.cpp b/clang/lib/Driver/ToolChains/MinGW.cpp
index 39d767795445dbe..5d7f8675daf8d28 100644
--- a/clang/lib/Driver/ToolChains/MinGW.cpp
+++ b/clang/lib/Driver/ToolChains/MinGW.cpp
@@ -249,7 +249,7 @@ void tools::MinGW::Linker::ConstructJob(Compilation &C, const JobAction &JA,
if (C.getDriver().IsFlangMode()) {
addFortranRuntimeLibraryPath(TC, Args, CmdArgs);
- addFortranRuntimeLibs(TC, CmdArgs);
+ addFortranRuntimeLibs(TC, Args, CmdArgs);
}
// TODO: Add profile stuff here
diff --git a/clang/lib/Driver/ToolChains/NetBSD.cpp b/clang/lib/Driver/ToolChains/NetBSD.cpp
index 7a1d4561c6f2f4f..beeea528f4c003c 100644
--- a/clang/lib/Driver/ToolChains/NetBSD.cpp
+++ b/clang/lib/Driver/ToolChains/NetBSD.cpp
@@ -321,7 +321,7 @@ void netbsd::Linker::ConstructJob(Compilation &C, const JobAction &JA,
// AddRunTimeLibs).
if (D.IsFlangMode()) {
addFortranRuntimeLibraryPath(ToolChain, Args, CmdArgs);
- addFortranRuntimeLibs(ToolChain, CmdArgs);
+ addFortranRuntimeLibs(ToolChain, Args, CmdArgs);
CmdArgs.push_back("-lm");
}
diff --git a/clang/lib/Driver/ToolChains/OpenBSD.cpp b/clang/lib/Driver/ToolChains/OpenBSD.cpp
index 16a311be31be7bc..5f531fc54ea5c0e 100644
--- a/clang/lib/Driver/ToolChains/OpenBSD.cpp
+++ b/clang/lib/Driver/ToolChains/OpenBSD.cpp
@@ -220,7 +220,7 @@ void openbsd::Linker::ConstructJob(Compilation &C, const JobAction &JA,
// AddRunTimeLibs).
if (D.IsFlangMode()) {
addFortranRuntimeLibraryPath(ToolChain, Args, CmdArgs);
- addFortranRuntimeLibs(ToolChain, CmdArgs);
+ addFortranRuntimeLibs(ToolChain, Args, CmdArgs);
if (Profiling)
CmdArgs.push_back("-lm_p");
else
diff --git a/clang/lib/Driver/ToolChains/Solaris.cpp b/clang/lib/Driver/ToolChains/Solaris.cpp
index 3d5a710842eca44..a2006076004a0af 100644
--- a/clang/lib/Driver/ToolChains/Solaris.cpp
+++ b/clang/lib/Driver/ToolChains/Solaris.cpp
@@ -225,7 +225,7 @@ void solaris::Linker::ConstructJob(Compilation &C, const JobAction &JA,
// these dependencies need to be listed before the C runtime below.
if (D.IsFlangMode()) {
addFortranRuntimeLibraryPath(getToolChain(), Args, CmdArgs);
- addFortranRuntimeLibs(getToolChain(), CmdArgs);
+ addFortranRuntimeLibs(getToolChain(), Args, CmdArgs);
CmdArgs.push_back("-lm");
}
if (Args.hasArg(options::OPT_fstack_protector) ||
diff --git a/flang/lib/Decimal/CMakeLists.txt b/flang/lib/Decimal/CMakeLists.txt
index 3116ff68ea2627e..95fc2188d2f6748 100644
--- a/flang/lib/Decimal/CMakeLists.txt
+++ b/flang/lib/Decimal/CMakeLists.txt
@@ -53,3 +53,26 @@ add_flang_library(FortranDecimal INSTALL_WITH_TOOLCHAIN
binary-to-decimal.cpp
decimal-to-binary.cpp
)
+
+if (DEFINED MSVC)
+ set(CMAKE_MSVC_RUNTIME_LIBRARY MultiThreaded)
+ add_flang_library(FortranDecimal.static INSTALL_WITH_TOOLCHAIN
+ binary-to-decimal.cpp
+ decimal-to-binary.cpp
+ )
+ set(CMAKE_MSVC_RUNTIME_LIBRARY MultiThreadedDLL)
+ add_flang_library(FortranDecimal.dynamic INSTALL_WITH_TOOLCHAIN
+ binary-to-decimal.cpp
+ decimal-to-binary.cpp
+ )
+ set(CMAKE_MSVC_RUNTIME_LIBRARY MultiThreadedDebug)
+ add_flang_library(FortranDecimal.static_debug INSTALL_WITH_TOOLCHAIN
+ binary-to-decimal.cpp
+ decimal-to-binary.cpp
+ )
+ set(CMAKE_MSVC_RUNTIME_LIBRARY MultiThreadedDebugDLL)
+ add_flang_library(FortranDecimal.dynamic_debug INSTALL_WITH_TOOLCHAIN
+ binary-to-decimal.cpp
+ decimal-to-binary.cpp
+ )
+endif()
\ No newline at end of file
diff --git a/flang/runtime/CMakeLists.txt b/flang/runtime/CMakeLists.txt
index f75daa373705f00..77882615c3dbce7 100644
--- a/flang/runtime/CMakeLists.txt
+++ b/flang/runtime/CMakeLists.txt
@@ -281,3 +281,26 @@ add_flang_library(FortranRuntime
INSTALL_WITH_TOOLCHAIN
)
+
+if (DEFINED MSVC)
+ set(CMAKE_MSVC_RUNTIME_LIBRARY MultiThreaded)
+ add_flang_library(FortranRuntime.static ${sources}
+ LINK_LIBS
+ FortranDecimal.static
+ INSTALL_WITH_TOOLCHAIN)
+ set(CMAKE_MSVC_RUNTIME_LIBRARY MultiThreadedDLL)
+ add_flang_library(FortranRuntime.dynamic ${sources}
+ LINK_LIBS
+ FortranDecimal.dynamic
+ INSTALL_WITH_TOOLCHAIN)
+ set(CMAKE_MSVC_RUNTIME_LIBRARY MultiThreadedDebug)
+ add_flang_library(FortranRuntime.static_debug ${sources}
+ LINK_LIBS
+ FortranDecimal.static_debug
+ INSTALL_WITH_TOOLCHAIN)
+ set(CMAKE_MSVC_RUNTIME_LIBRARY MultiThreadedDebugDLL)
+ add_flang_library(FortranRuntime.dynamic_debug ${sources}
+ LINK_LIBS
+ FortranDecimal.dynamic_debug
+ INSTALL_WITH_TOOLCHAIN)
+endif()
diff --git a/flang/runtime/FortranMain/CMakeLists.txt b/flang/runtime/FortranMain/CMakeLists.txt
index fe0d607c3f1a951..9fff1317d386d59 100644
--- a/flang/runtime/FortranMain/CMakeLists.txt
+++ b/flang/runtime/FortranMain/CMakeLists.txt
@@ -1,3 +1,21 @@
add_flang_library(Fortran_main STATIC INSTALL_WITH_TOOLCHAIN
Fortran_main.c
)
+if (DEFINED MSVC)
+ set(CMAKE_MSVC_RUNTIME_LIBRARY MultiThreaded)
+ add_flang_library(Fortran_main.static STATIC INSTALL_WITH_TOOLCHAIN
+ Fortran_main.c
+ )
+ set(CMAKE_MSVC_RUNTIME_LIBRARY MultiThreadedDLL)
+ add_flang_library(Fortran_main.dynamic STATIC INSTALL_WITH_TOOLCHAIN
+ Fortran_main.c
+ )
+ set(CMAKE_MSVC_RUNTIME_LIBRARY MultiThreadedDebug)
+ add_flang_library(Fortran_main.static_debug STATIC INSTALL_WITH_TOOLCHAIN
+ Fortran_main.c
+ )
+ set(CMAKE_MSVC_RUNTIME_LIBRARY MultiThreadedDebugDLL)
+ add_flang_library(Fortran_main.dynamic_debug STATIC INSTALL_WITH_TOOLCHAIN
+ Fortran_main.c
+ )
+endif()
diff --git a/flang/test/Driver/driver-help-hidden.f90 b/flang/test/Driver/driver-help-hidden.f90
index 6d399f1d179a022..37c59221c3e0bc0 100644
--- a/flang/test/Driver/driver-help-hidden.f90
+++ b/flang/test/Driver/driver-help-hidden.f90
@@ -59,6 +59,8 @@
! CHECK-NEXT: -flto=jobserver Enable LTO in 'full' mode
! CHECK-NEXT: -flto=<value> Set LTO mode
! CHECK-NEXT: -flto Enable LTO in 'full' mode
+! CHECK-NEXT: -fms-runtime-lib=<value>
+! CHECK-NEXT: Select Windows run-time library
! CHECK-NEXT: -fno-alias-analysis Do not pass alias information on to LLVM (default for unoptimized builds)
! CHECK-NEXT: -fno-automatic Implies the SAVE attribute for non-automatic local objects in subprograms unless RECURSIVE
! CHECK-NEXT: -fno-color-diagnostics Disable colors in diagnostics
diff --git a/flang/test/Driver/driver-help.f90 b/flang/test/Driver/driver-help.f90
index 31c9caa32ea8292..2f81ec260ec964f 100644
--- a/flang/test/Driver/driver-help.f90
+++ b/flang/test/Driver/driver-help.f90
@@ -51,6 +51,8 @@
! HELP-NEXT: -flto=jobserver Enable LTO in 'full' mode
! HELP-NEXT: -flto=<value> Set LTO mode
! HELP-NEXT: -flto Enable LTO in 'full' mode
+! HELP-NEXT: -fms-runtime-lib=<value>
+! HELP-NEXT: Select Windows run-time library
! HELP-NEXT: -fno-alias-analysis Do not pass alias information on to LLVM (default for unoptimized builds)
! HELP-NEXT: -fno-automatic Implies the SAVE attribute for non-automatic local objects in subprograms unless RECURSIVE
! HELP-NEXT: -fno-color-diagnostics Disable colors in diagnostics
diff --git a/flang/test/Driver/linker-flags.f90 b/flang/test/Driver/linker-flags.f90
index e4d713df499b7d0..63451eeab62c90d 100644
--- a/flang/test/Driver/linker-flags.f90
+++ b/flang/test/Driver/linker-flags.f90
@@ -12,11 +12,13 @@
! RUN: %flang -### --target=x86_64-unknown-haiku %S/Inputs/hello.f90 2>&1 | FileCheck %s --check-prefixes=CHECK,HAIKU
! RUN: %flang -### --target=x86_64-windows-gnu %S/Inputs/hello.f90 2>&1 | FileCheck %s --check-prefixes=CHECK,MINGW
-! NOTE: Clang's driver library, clangDriver, usually adds 'libcmt' and
-! 'oldnames' on Windows, but they are not needed when compiling
-! Fortran code and they might bring in additional dependencies.
-! Make sure they're not added.
-! RUN: %flang -### --target=aarch64-windows-msvc -fuse-ld= %S/Inputs/hello.f90 2>&1 | FileCheck %s --check-prefixes=CHECK,MSVC --implicit-check-not libcmt --implicit-check-not oldnames
+! NOTE: Clang's driver library, clangDriver, usually adds 'oldnames' on Windows,
+! but it is not needed when compiling Fortran code and they might bring in
+! additional dependencies. Make sure its not added.
+! RUN: %flang -### --target=aarch64-windows-msvc -fuse-ld= %S/Inputs/hello.f90 2>&1 | FileCheck %s --check-prefixes=CHECK,MSVC --implicit-check-not oldnames
+! RUN: %flang -### --target=aarch64-windows-msvc -fms-runtime-lib=static_dbg -fuse-ld= %S/Inputs/hello.f90 2>&1 | FileCheck %s --check-prefixes=CHECK,MSVC-DEBUG --implicit-check-not oldnames
+! RUN: %flang -### --target=aarch64-windows-msvc -fms-runtime-lib=dll -fuse-ld= %S/Inputs/hello.f90 2>&1 | FileCheck %s --check-prefixes=CHECK,MSVC-DLL --implicit-check-not oldnames
+! RUN: %flang -### --target=aarch64-windows-msvc -fms-runtime-lib=dll_dbg -fuse-ld= %S/Inputs/hello.f90 2>&1 | FileCheck %s --check-prefixes=CHECK,MSVC-DLL-DEBUG --implicit-check-not oldnames
! Compiler invocation to generate the object file
! CHECK-LABEL: {{.*}} "-emit-obj"
@@ -52,8 +54,37 @@
! (lld-)link.exe on Windows platforms. The suffix may not be added
! when the executable is not found or on non-Windows platforms.
! MSVC-LABEL: link
-! MSVC-SAME: Fortran_main.lib
-! MSVC-SAME: FortranRuntime.lib
-! MSVC-SAME: FortranDecimal.lib
+! MSVC-SAME: /DEFAULTLIB:clang_rt.builtins-aarch64.lib
+! MSVC-SAME: /DEFAULTLIB:libcmt
+! MSVC-SAME: Fortran_main.static.lib
+! MSVC-SAME: FortranRuntime.static.lib
+! MSVC-SAME: FortranDecimal.static.lib
! MSVC-SAME: /subsystem:console
! MSVC-SAME: "[[object_file]]"
+
+! MSVC-DEBUG-LABEL: link
+! MSVC-DEBUG-SAME: /DEFAULTLIB:clang_rt.builtins-aarch64.lib
+! MSVC-DEBUG-SAME: /DEFAULTLIB:libcmtd
+! MSVC-DEBUG-SAME: Fortran_main.static_debug.lib
+! MSVC-DEBUG-SAME: FortranRuntime.static_debug.lib
+! MSVC-DEBUG-SAME: FortranDecimal.static_debug.lib
+! MSVC-DEBUG-SAME: /subsystem:console
+! MSVC-DEBUG-SAME: "[[object_file]]"
+
+! MSVC-DLL-LABEL: link
+! MSVC-DLL-SAME: /DEFAULTLIB:clang_rt.builtins-aarch64.lib
+! MSVC-DLL-SAME: /DEFAULTLIB:msvcrt
+! MSVC-DLL-SAME: Fortran_main.dynamic.lib
+! MSVC-DLL-SAME: FortranRuntime.dynamic.lib
+! MSVC-DLL-SAME: FortranDecimal.dynamic.lib
+! MSVC-DLL-SAME: /subsystem:console
+! MSVC-DLL-SAME: "[[object_file]]"
+
+! MSVC-DLL-DEBUG-LABEL: link
+! MSVC-DLL-DEBUG-SAME: /DEFAULTLIB:clang_rt.builtins-aarch64.lib
+! MSVC-DLL-DEBUG-SAME: /DEFAULTLIB:msvcrtd
+! MSVC-DLL-DEBUG-SAME: Fortran_main.dynamic_debug.lib
+! MSVC-DLL-DEBUG-SAME: FortranRuntime.dynamic_debug.lib
+! MSVC-DLL-DEBUG-SAME: FortranDecimal.dynamic_debug.lib
+! MSVC-DLL-DEBUG-SAME: /subsystem:console
+! MSVC-DLL-DEBUG-SAME: "[[object_file]]"
|
DavidTruby
requested review from
rnk,
mstorsjo,
zmodem,
banach-space and
jeanPerier
|
@bradking I don't appear to be able to add you as a reviewer but it might be worth you having a look at this too, thanks! |
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
| @@ -53,3 +53,26 @@ add_flang_library(FortranDecimal INSTALL_WITH_TOOLCHAIN | |||
| binary-to-decimal.cpp | |||
| decimal-to-binary.cpp | |||
| ) | |||
|
|
|||
| if (DEFINED MSVC) | |||
| set(CMAKE_MSVC_RUNTIME_LIBRARY MultiThreaded) | |||
There was a problem hiding this comment.
Instead of redefining CMAKE_MSVC_RUNTIME_LIBRARY repeatedly, if you really want to set it specifically for one library, it's better to set the MSVC_RUNTIME_LIBRARY target property instead - see https://cmake.org/cmake/help/latest/prop_tgt/MSVC_RUNTIME_LIBRARY.html.
There was a problem hiding this comment.
Yeah I tried this but inside add_flang_library these are made into custom targets (meaning the target property doesn’t do anything) and I’m not entirely sure why so I didn’t want to mess with it too much. I’ll give it another try
There was a problem hiding this comment.
add_flang_library eventually ends up in llvm/cmake/modules/AddLLVM.cmake's llvm_add_library which calls add_library(${name} STATIC ...). All CMAKE_MSVC_RUNTIME_LIBRARY does is initialize the MSVC_RUNTIME_LIBRARY property on that target when it is created.
You should be able to do
add_flang_library(FortranDecimal.static ...)
set_property(TARGET FortranDecimal.static PROPERTY MSVC_RUNTIME_LIBRARY MultiThreaded)
| @@ -53,3 +53,26 @@ add_flang_library(FortranDecimal INSTALL_WITH_TOOLCHAIN | |||
| binary-to-decimal.cpp | |||
| decimal-to-binary.cpp | |||
| ) | |||
|
|
|||
| if (DEFINED MSVC) | |||
| set(CMAKE_MSVC_RUNTIME_LIBRARY MultiThreaded) | |||
There was a problem hiding this comment.
add_flang_library eventually ends up in llvm/cmake/modules/AddLLVM.cmake's llvm_add_library which calls add_library(${name} STATIC ...). All CMAKE_MSVC_RUNTIME_LIBRARY does is initialize the MSVC_RUNTIME_LIBRARY property on that target when it is created.
You should be able to do
add_flang_library(FortranDecimal.static ...)
set_property(TARGET FortranDecimal.static PROPERTY MSVC_RUNTIME_LIBRARY MultiThreaded)| CmdArgs.push_back("Fortran_main.dynamic.lib"); | ||
| CmdArgs.push_back("FortranRuntime.dynamic.lib"); | ||
| CmdArgs.push_back("FortranDecimal.dynamic.lib"); | ||
| break; |
There was a problem hiding this comment.
From #70833 (comment):
I think you're probably right in the linked issue that it'd be better to add defaultlib directives to the object files, but that appears to be quite difficult as we'd need to track the attributes all the way through our MLIR lowering, so as a (hopefully temporary) shortcut I have just passed the libraries on the link line.
This temporary approach will actually make things harder for CMake to support flang-new. In order to support mixed-language (e.g., Fortran and C++) binaries we detect the implicit link directories and libraries that each compiler driver passes to the linker when used to drive linking. Then if we have to link using a different language's tooling, we can add them explicitly. We don't typically do that for the MSVC ABI though because the set of runtime libraries varies with the CRT choice and the defaultlib directives in object files handle it automatically anyway. Currently CMake is working around the lack of defaultlib directives for flang-new by using the implicit-lib-detection approach. Once the implicitly linked runtime libraries vary with the CRT, we would need a lot of dedicated non-trivial infrastructure to handle all the MSVC_RUNTIME_LIBRARY variants, and I'm not sure it's possible in all cases.
Can you instead add these four CRT-specific libraries as defaultlib directives in all object files, and add the more detailed conditions to remove unnecessary libraries later? Since they are all static .lib files, unused directives may not hurt.
There was a problem hiding this comment.
Is there a way to add defaultlib directives using a CLI tool, or do they have to be added when the compiler creates the object file? The main issue is we have an MLIR step in between flang and LLVM IR and I don't believe MLIR supports the attributes for setting these defaultlib directives yet, so implimenting that might be quite a lot of work... It's not so much that we would have difficulty adding the specific defaultlib directive we need, as that we'd have an issue adding any of them.
There was a problem hiding this comment.
Having had a look, I think we can replicate what clang -cc1's --dependent-lib= setting does possibly
EDIT: this looks quite complicated, but I think is what we need to do in the long run. Do you think we could merge this patch as at least an improvement, and I will look at adding this option to do things correctly?
There was a problem hiding this comment.
I'm not aware of any easy way to add defaultlib directives after-the-fact.
I think it's okay to merge this PR's approach as a temporary solution. It does fix the empty-program example in #68017 work, and provide the Fortran runtime library variants. However, I'm not sure how well CMake will be able to support this until the defaultlib part is added.
BTW, the Fixes #68017 note in the PR description is no longer accurate. It's not fully fixed yet.
| CmdArgs.push_back("FortranDecimal.lib"); | ||
| CmdArgs.push_back(Args.MakeArgString( | ||
| "/DEFAULTLIB:" + TC.getCompilerRTBasename(Args, "builtins"))); | ||
| unsigned RTOptionID = options::OPT__SLASH_MT; |
There was a problem hiding this comment.
As of LLVM 17.0's distribution, the Fortran runtime libraries are built with msvcrt, so I think the current default is actually /MD. Since this wasn't really modeled before, and CMake will be taught to pass one of the four flags explicitly anyway, changing the default may not matter, but it's something to be aware of.
There was a problem hiding this comment.
I think we should take the opportunity to match what clang's default behaviour is here, which is to pass /MT. I think having the two do something different is surprising at the moment and should be changed.
There was a problem hiding this comment.
Yes, changing the default to match Clang makes sense.
There was a problem hiding this comment.
In local testing, flang-new -fms-runtime-lib=static foo.f90 -v, where foo.f90 is an empty program statement, fails with a bunch of unresolved CRT symbols.
There was a problem hiding this comment.
Could you share the missing symbols? If it's something like __udivti3 then that's an issue with not finding compiler-rt (those symbols are for 128 bit types and not contained in the MSVC rt libs of any flavour), see #25679. If so, there's perhaps a chance that things work with flang-new -DAVOID_NATIVE_UINT128_T=1...
There was a problem hiding this comment.
I should be adding the missing compiler-rt libs in this patch too. I don’t see any missing symbols in any of the configurations when testing an empty program locally. Could you share the error you’re seeing?
There was a problem hiding this comment.
The errors were due to #70833 (review) because the runtime library variants were not being built with the correct CRT themselves. After switching back to the CMAKE_MSVC_RUNTIME_LIBRARY the problem is resolved.
|
Do we really need to have all 4 variants of the 3 fortran runtime libraries? That's a lot of complexity. Can we pare it down to just static/dynamic? It's also sometimes possible to generate code that works in both the static and dynamic context, depending on what is in those libraries. We don't create 4 variants of clang_rt.builtins, for examle. |
From glancing at the fortran runtime code, I think the answer is probably "no". There is too much C++ standard library usage. If you wish to avoid this build complexity, you may consider writing code in the STL-less style that is used for C++ code in the sanitizers in compiler-rt. |
I don't think we can avoid it if we want to allow anyone to link flang-generated object files into a C/C++ application. I don't think we could even get it down to static/dynamic reliably without committing to not only not using the STL but not using any C/C++ functions that might call into the runtime (as compiler-rt builtins does). I don't think that's a route we want to go down with the flang runtime; I think we'd generally put build complexity secondary to code complexity (I don't speak for every flang developer of course) I'd personally add that I particularly feel this when only one platform is requiring the extra build complexity.. |
flang/runtime/CMakeLists.txt
Outdated
| LINK_LIBS | ||
| FortranDecimal.static | ||
| INSTALL_WITH_TOOLCHAIN) | ||
| set_property(TARGET FortranRuntime.static PROPERTY MSVC_RUNTIME_LIBRARY MultiThreaded) |
There was a problem hiding this comment.
After local testing, it seems my earlier advice to set the MSVC_RUNTIME_LIBRARY property directly instead of using CMAKE_MSVC_RUNTIME_LIBRARY was incorrect. LLVM's CMake infrastructure has options for using object libraries, in which case the compilation might not actually happen in the targets we name here. Please switch back to the set(CMAKE_MSVC_RUNTIME_LIBRARY ...) pattern.
| .Case("static_dbg", options::OPT__SLASH_MTd) | ||
| .Case("dll", options::OPT__SLASH_MD) | ||
| .Case("dll_dbg", options::OPT__SLASH_MDd) | ||
| .Default(options::OPT__SLASH_MT); |
There was a problem hiding this comment.
The switch accepts names static,static_dbg,dll,dbg_dll. Should we use matching names for the FortranRuntime.*.lib variants?
There was a problem hiding this comment.
Hmm. Now that I see those names on disk after building from your update, file names like FortranRuntime.dll.lib might be confusing since they do not actually have a corresponding FortranRuntime.dll. Maybe .dynamic was better.
| if (DEFINED MSVC) | ||
| add_flang_library(Fortran_main.static STATIC INSTALL_WITH_TOOLCHAIN | ||
| Fortran_main.c | ||
| ) |
There was a problem hiding this comment.
The style elsewhere seems to use 2 spaces for indentation.
|
|
||
| if (DEFINED MSVC) | ||
| set(CMAKE_MSVC_RUNTIME_LIBRARY MultiThreaded) | ||
| add_flang_library(FortranRuntime.static ${sources} |
There was a problem hiding this comment.
When targeting the MSVC ABI, the plain FortranRuntime library added above should be excluded. Only the per-CRT variants should exist, because choosing a CRT variant is not optional.
There was a problem hiding this comment.
The plain FortranRuntime library is linked by the Runtime tests, which need to be built against whatever the user built LLVM with, which we can't necessarily find out that easily I think. It should not have INSTALL_WITH_TOOLCHAIN set though, I'll remove that
|
@bradking do you think this is ok to merge now? |
|
@DavidTruby please see my above retraction of the suggestion to rename |
Ah, I missed that, I'll make that change back |
|
I've also started the work to get linker option directives (like "/DEFAULTLIB") added to MLIR's LLVMIR dialect so that we can do this the correct way: see #71720. I'd still like to merge this patch first though for the incremental improvement it does provide. Then once the MLIR support is merged I can switch over to using that and this should be resolved in the way that would help CMake's support for flang. |
Currently flang's runtime libraries are only built for the specific CRT that LLVM itself was built against. This patch adds the cmake logic for building a separate runtime for each CRT configuration and adds a flag for selecting a CRT configuration to link against.
DavidTruby
force-pushed
the
flang-crt
branch
from
e26e635
to
4605714
Compare
…0833) Currently flang's runtime libraries are only built for the specific CRT that LLVM itself was built against. This patch adds the cmake logic for building a separate runtime for each CRT configuration and adds a flag for selecting a CRT configuration to link against.
Currently flang's runtime libraries are only built for the specific CRT
that LLVM itself was built against. This patch adds the cmake logic for
building a separate runtime for each CRT configuration and adds a flag
for selecting a CRT configuration to link against.