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

[flang][Driver] Support -pthread to the frontend. #75739

Closed
wants to merge 1 commit into from

Conversation

ergawy
Copy link
Member

@ergawy ergawy commented Dec 17, 2023

Adds -pthread option to flang. Since the GNU toolchain already adds the required linker flag, we only need to declare FlangOption as one of the supported options for -pthread.

@llvmbot llvmbot added clang Clang issues not falling into any other category flang:driver flang Flang issues not falling into any other category labels Dec 17, 2023
@llvmbot
Copy link
Collaborator

llvmbot commented Dec 17, 2023

@llvm/pr-subscribers-clang

@llvm/pr-subscribers-flang-driver

Author: Kareem Ergawy (ergawy)

Changes

Adds -pthread option to flang. Since the GNU toolchain already adds the required linker flag, we only need to declare FlangOption as one of the supported options for -pthread.


Full diff: https://github.com/llvm/llvm-project/pull/75739.diff

2 Files Affected:

  • (modified) clang/include/clang/Driver/Options.td (+1-1)
  • (modified) flang/test/Driver/dynamic-linker.f90 (+2-1)
diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td
index 1b02087425b751..b8b8d476413982 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -5297,7 +5297,7 @@ def pthreads : Flag<["-"], "pthreads">;
 defm pthread : BoolOption<"", "pthread",
   LangOpts<"POSIXThreads">, DefaultFalse,
   PosFlag<SetTrue, [], [ClangOption], "Support POSIX threads in generated code">,
-  NegFlag<SetFalse>, BothFlags<[], [ClangOption, CC1Option]>>;
+  NegFlag<SetFalse>, BothFlags<[], [ClangOption, CC1Option, FlangOption]>>;
 def pie : Flag<["-"], "pie">, Group<Link_Group>;
 def static_pie : Flag<["-"], "static-pie">, Group<Link_Group>;
 def read__only__relocs : Separate<["-"], "read_only_relocs">;
diff --git a/flang/test/Driver/dynamic-linker.f90 b/flang/test/Driver/dynamic-linker.f90
index df119c22a2ea51..57a2af01aadff7 100644
--- a/flang/test/Driver/dynamic-linker.f90
+++ b/flang/test/Driver/dynamic-linker.f90
@@ -1,7 +1,7 @@
 ! Verify that certain linker flags are known to the frontend and are passed on
 ! to the linker.
 
-! RUN: %flang -### --target=x86_64-linux-gnu -rpath /path/to/dir -shared \
+! RUN: %flang -### --target=x86_64-linux-gnu -rpath /path/to/dir -shared -pthread \
 ! RUN:     -static %s 2>&1 | FileCheck \
 ! RUN:     --check-prefixes=GNU-LINKER-OPTIONS %s
 ! RUN: %flang -### --target=x86_64-windows-msvc -rpath /path/to/dir -shared \
@@ -13,6 +13,7 @@
 ! GNU-LINKER-OPTIONS-SAME: "-shared"
 ! GNU-LINKER-OPTIONS-SAME: "-static"
 ! GNU-LINKER-OPTIONS-SAME: "-rpath" "/path/to/dir"
+! GNU-LINKER-OPTIONS-SAME: "-lpthread"
 
 ! For MSVC, adding -static does not add any additional linker options.
 ! MSVC-LINKER-OPTIONS: "{{.*}}link{{(.exe)?}}"

Adds `-pthread` option to flang. Since the GNU toolchain already adds
the required linker flag, we only need to declare `FlangOption` as one
of the supported options for `-pthread`.
@ergawy ergawy force-pushed the flang_driver_support_pthread branch from 2880ebc to 395b56f Compare December 17, 2023 15:14
@banach-space
Copy link
Contributor

Hi @ergawy , thanks for this contribution! Could you add a test that would demonstrate compilation failing without -pthread?

@banach-space banach-space self-requested a review December 18, 2023 08:06
@ergawy
Copy link
Member Author

ergawy commented Dec 22, 2023

Hi @ergawy , thanks for this contribution! Could you add a test that would demonstrate compilation failing without -pthread?

Thanks for the suggestion. Actually I failed to do that 😆. After looking into it, seems that for the GNU toolchain, the -pthread flag would be redundant. That reason is that the pthread API is defined by libc which is linked in by the driver by default.

I think I will abandon this PR then. Just need to double check once more.

@ergawy
Copy link
Member Author

ergawy commented Dec 22, 2023

Abandoning this PR since for the GNU toolchain there is no need to explicitly link with pthread to use the API.

@ergawy ergawy closed this Dec 22, 2023
@ergawy ergawy deleted the flang_driver_support_pthread branch December 22, 2023 19:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang Clang issues not falling into any other category flang:driver flang Flang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants