[flang][driver] Add support for -isysroot in the frontend #77365
Conversation
If DEFAULT_SYSROOT is not specfied when building flang, then the -isysroot flag is needed to link binaries against system libraries on Darwin. It's also needed when linking against a non-default sysroot.
llvmbot
added
clang
|
@llvm/pr-subscribers-flang-semantics @llvm/pr-subscribers-clang Author: Leandro Lupori (luporl) ChangesIf DEFAULT_SYSROOT is not specfied when building flang, then the Full diff: https://github.com/llvm/llvm-project/pull/77365.diff 4 Files Affected:
diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td
index 6aff37f1336871..f42e9c7eb92a67 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -4265,7 +4265,7 @@ def iquote : JoinedOrSeparate<["-"], "iquote">, Group<clang_i_Group>,
Visibility<[ClangOption, CC1Option]>,
HelpText<"Add directory to QUOTE include search path">, MetaVarName<"<directory>">;
def isysroot : JoinedOrSeparate<["-"], "isysroot">, Group<clang_i_Group>,
- Visibility<[ClangOption, CC1Option]>,
+ Visibility<[ClangOption, CC1Option, FlangOption]>,
HelpText<"Set the system root directory (usually /)">, MetaVarName<"<dir>">,
MarshallingInfoString<HeaderSearchOpts<"Sysroot">, [{"/"}]>;
def isystem : JoinedOrSeparate<["-"], "isystem">, Group<clang_i_Group>,
diff --git a/flang/test/Driver/driver-help-hidden.f90 b/flang/test/Driver/driver-help-hidden.f90
index 9a11a7a571ffcc..a4444ca043d0ea 100644
--- a/flang/test/Driver/driver-help-hidden.f90
+++ b/flang/test/Driver/driver-help-hidden.f90
@@ -111,6 +111,7 @@
! CHECK-NEXT: -g Generate source-level debug information
! CHECK-NEXT: --help-hidden Display help for hidden options
! CHECK-NEXT: -help Display available options
+! CHECK-NEXT: -isysroot <dir> Set the system root directory (usually /)
! CHECK-NEXT: -I <dir> Add directory to the end of the list of include search paths
! CHECK-NEXT: -L <dir> Add directory to library search path
! CHECK-NEXT: -march=<value> For a list of available architectures for the target use '-mcpu=help'
diff --git a/flang/test/Driver/driver-help.f90 b/flang/test/Driver/driver-help.f90
index e0e74dc56f331e..07189264104592 100644
--- a/flang/test/Driver/driver-help.f90
+++ b/flang/test/Driver/driver-help.f90
@@ -97,6 +97,7 @@
! HELP-NEXT: -g Generate source-level debug information
! HELP-NEXT: --help-hidden Display help for hidden options
! HELP-NEXT: -help Display available options
+! HELP-NEXT: -isysroot <dir> Set the system root directory (usually /)
! HELP-NEXT: -I <dir> Add directory to the end of the list of include search paths
! HELP-NEXT: -L <dir> Add directory to library search path
! HELP-NEXT: -march=<value> For a list of available architectures for the target use '-mcpu=help'
diff --git a/flang/test/Driver/isysroot.f90 b/flang/test/Driver/isysroot.f90
new file mode 100644
index 00000000000000..70d2fc0345ce50
--- /dev/null
+++ b/flang/test/Driver/isysroot.f90
@@ -0,0 +1,12 @@
+! Verify that the -isysroot flag is known to the frontend and, on Darwin,
+! is passed on to the linker.
+
+! RUN: %flang -### --target=aarch64-apple-darwin -isysroot /path/to/sysroot \
+! RUN: %s 2>&1 | FileCheck %s --check-prefix=CHECK-DARWIN
+! RUN: %flang -### --target=aarch64-linux-gnu -isysroot /path/to/sysroot \
+! RUN: %s 2>&1 | FileCheck %s --check-prefix=CHECK-LINUX
+
+! CHECK-DARWIN: "{{.*}}/ld" {{.*}}"-syslibroot" "/path/to/sysroot"
+! Unused on Linux.
+! CHECK-LINUX: warning: argument unused during compilation: '-isysroot /path/to/sysroot'
+! CHECK-LINUX-NOT: /path/to/sysroot
|
|
Thanks - could you also update the driver docs? |
|
✅ With the latest revision this PR passed the Python code formatter. |
|
Thanks for the review and all the suggestions! |
|
If the compiler is built with |
I expected |
This behavior is inherited from clang: https://github.com/llvm/llvm-project/blob/main/clang/lib/Driver/ToolChains/Darwin.cpp#L430 Only I think
An alternative would be to also support My preference is to add support for |
|
Thank you for this summary, @luporl !
My biggest concern would be your 2nd point above - I'd rather avoid. Like you said - folks might depend on the current behaviour.
You could also use
Wouldn't the behaviour of
Can you remind me the benefits of using |
Ok, I'll take at look at that.
The behavior "shouldn't" change, but the sysroot used when linking
It seems to me that Apple prefers to use Also, when grepping through CMake generated files (for llvm), I saw that they use Apart from that, the other difference that I'm aware is that on non-Darwin targets |
Good, so in practice the env variable won’t be affecting this test.
Alignment with the official guidelines would be nice. If possible :)
Sounds like in practice it wouldn’t make much difference whether it’s -isysroot or -sysroot? |
Right, but adding support for -sysroot too would have 2 main benefits:
My plan is the following:
Does it look good? |
@luporl Thanks a lot. Your proposed plan looks good to me. |
If DEFAULT_SYSROOT is not specfied when building flang, then the -isysroot flag is needed to link binaries against system libraries on Darwin. It's also needed when linking against a non-default sysroot.
|
It looks like I noticed, however, that using Interestingly enough, some other flags also show the same issue, such as My hunch is that option aliases are not working properly with flang-new, when Anyone knows if this is expected and |
|
Thank you for your continued effort to improve this!
IIRC, when |
|
#80728 opened to track this issue. |
If DEFAULT_SYSROOT is not specfied when building flang, then the
-isysroot flag is needed to link binaries against system libraries
on Darwin. It's also needed when linking against a non-default
sysroot.