mmuetzel added a comment. In D126291#3572777 <https://reviews.llvm.org/D126291#3572777>, @rovka wrote:
> Moved the check for `-flang-experimental-exec` into > addFortranRuntimeLibraryPath, so it affects all the toolchains. @awarzynski > does this look like a good idea? If moving that check to inside that function is ok, should the same check be added to `addFortranRuntimeLibs`, too? ================ Comment at: clang/lib/Driver/ToolChains/MSVC.cpp:134-135 + if (C.getDriver().IsFlangMode()) { + tools::addFortranRuntimeLibraryPath(TC, Args, CmdArgs); + tools::addFortranRuntimeLibs(TC, CmdArgs); + ---------------- Nit: The other toolchain files (Gnu and Darwin) don't explicitly specify the `tools` namespace for these functions. Is there a preferred coding style when it comes to this? Same for the MinGW toolchain file. ================ Comment at: clang/lib/Driver/ToolChains/MinGW.cpp:224 + tools::addFortranRuntimeLibs(TC, CmdArgs); + } + ---------------- MinGW behaves similarly to GNU in many respects. The GNU toolchain file adds `CmdArgs.push_back("-lm");` here as well. I didn't need that for the simple "Hello World" program. But that didn't invoke any math functions... Do we need to add that flag here, too? ================ Comment at: flang/test/Driver/linker-flags-windows.f90:6 +! NOTE: The additional linker flags tested here are currently specified in +! clang/lib/Driver/Toolchains/MSVC.cpp. +! REQUIRES: windows-msvc ---------------- Since this test only applies to MSVC toolchains (not to MinGW), should the file be renamed to, e.g., `linker-flags-msvc.f90`? Should there be a similar test for MinGW? Which `REQUIRES` would that need? Could maybe look like this (pending the correct `REQUIRES` line): ``` ! Verify that the Fortran runtime libraries are present in the linker ! invocation. These libraries are added on top of other standard runtime ! libraries that the Clang driver will include. ! NOTE: The additional linker flags tested here are currently specified in ! clang/lib/Driver/Toolchains/MinGW.cpp. ! REQUIRES: windows-mingw !------------ ! RUN COMMAND !------------ ! RUN: %flang -### -flang-experimental-exec %S/Inputs/hello.f90 2>&1 | FileCheck %s !---------------- ! EXPECTED OUTPUT !---------------- ! Compiler invocation to generate the object file ! CHECK-LABEL: {{.*}} "-emit-obj" ! CHECK-SAME: "-o" "[[object_file:.*]]" {{.*}}Inputs/hello.f90 ! Linker invocation to generate the executable ! CHECK-SAME: "[[object_file]]" ! CHECK-SAME: -lFortran_main ! CHECK-SAME: -lFortranRuntime ! CHECK-SAME: -lFortranDecimal ! CHECK-SAME: -lm ``` ================ Comment at: flang/test/Driver/linker-flags-windows.f90:9 + +! RUN: %flang -### %S/Inputs/hello.f90 2>&1 | FileCheck %s + ---------------- This will probably need to be changed like this ``` -! RUN: %flang -### %S/Inputs/hello.f90 2>&1 | FileCheck %s +! RUN: %flang -### -flang-experimental-exec %S/Inputs/hello.f90 2>&1 | FileCheck %s ``` CHANGES SINCE LAST ACTION https://reviews.llvm.org/D126291/new/ https://reviews.llvm.org/D126291 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits