Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Driver] Link Flang runtime on Solaris #65644

Merged
merged 3 commits into from
Oct 18, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
8 changes: 8 additions & 0 deletions clang/lib/Driver/ToolChains/Solaris.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -223,6 +223,14 @@ void solaris::Linker::ConstructJob(Compilation &C, const JobAction &JA,
getToolChain().AddCXXStdlibLibArgs(Args, CmdArgs);
CmdArgs.push_back("-lm");
}
// Additional linker set-up and flags for Fortran. This is required in order
// to generate executables. As Fortran runtime depends on the C runtime,
// these dependencies need to be listed before the C runtime below.
if (D.IsFlangMode()) {
addFortranRuntimeLibraryPath(getToolChain(), Args, CmdArgs);
addFortranRuntimeLibs(getToolChain(), CmdArgs);
CmdArgs.push_back("-lm");
}
if (Args.hasArg(options::OPT_fstack_protector) ||
Args.hasArg(options::OPT_fstack_protector_strong) ||
Args.hasArg(options::OPT_fstack_protector_all)) {
Expand Down
1 change: 1 addition & 0 deletions flang/test/Driver/linker-flags.f90
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

! RUN: %flang -### -target ppc64le-linux-gnu %S/Inputs/hello.f90 2>&1 | FileCheck %s --check-prefixes=CHECK,GNU
! RUN: %flang -### -target aarch64-apple-darwin %S/Inputs/hello.f90 2>&1 | FileCheck %s --check-prefixes=CHECK,DARWIN
! RUN: %flang -### -target sparc-sun-solaris2.11 %S/Inputs/hello.f90 2>&1 | FileCheck %s --check-prefixes=CHECK,GNU
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Solaris is not GNU ;-) Please use a dedicated prefix.

Copy link
Collaborator Author

@rorth rorth Sep 8, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

True, but how many more copies of the same patterns do we want? Add FreeBSD, OpenBSD, ... whenever they arrive? Isn't the common pattern <linker> <object file> <flang rt libs> with or without -lm? How about putting the first under one label and the -lm part under another, used where appropriate?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, reducing duplication would be nice. Could you propose something?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@banach-space How should we proceed with this one? Really introduce separate labels for each OS, even if the checks are identical? Or consolidate labels as I suggested?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would consolidate like you suggested. Perhaps the common prefix cold be "ALL-RT-LIBS" or "FLANG-RT-LIBS"? Suggestions welcome.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add FreeBSD, OpenBSD, ... whenever they arrive?

Bumping into this PR that'll be happening much sooner for all of the *BSD's now.

! 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
Expand Down