[FLANG] allow -fopenmp= #86816
Merged
[FLANG] allow -fopenmp= #86816
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This enables the -fopenmp=<library> option to the set of options supported by flang. The generated arguments for the FC1 compilation will appear in a slightly different order, so one test had to be updated to be less sensitive to order of the arguments.
llvmbot
added
clang
|
@llvm/pr-subscribers-clang-driver @llvm/pr-subscribers-flang-driver Author: Mats Petersson (Leporacanthicus) ChangesThis enables the -fopenmp=<library> option to the set of options supported by flang. The generated arguments for the FC1 compilation will appear in a slightly different order, so one test had to be updated to be less sensitive to order of the arguments. Full diff: https://github.com/llvm/llvm-project/pull/86816.diff 4 Files Affected:
diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td
index aca8c9b0d5487a..3e78dada51a1b3 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -3403,7 +3403,8 @@ defm openmp_extensions: BoolFOption<"openmp-extensions",
"Enable all Clang extensions for OpenMP directives and clauses">,
NegFlag<SetFalse, [NoArgumentUnused], [ClangOption, CC1Option],
"Disable all Clang extensions for OpenMP directives and clauses">>;
-def fopenmp_EQ : Joined<["-"], "fopenmp=">, Group<f_Group>;
+def fopenmp_EQ : Joined<["-"], "fopenmp=">, Group<f_Group>,
+ Visibility<[ClangOption, CC1Option, FlangOption, FC1Option]>;
def fopenmp_use_tls : Flag<["-"], "fopenmp-use-tls">, Group<f_Group>,
Flags<[NoArgumentUnused, HelpHidden]>;
def fnoopenmp_use_tls : Flag<["-"], "fnoopenmp-use-tls">, Group<f_Group>,
diff --git a/clang/lib/Driver/ToolChains/Flang.cpp b/clang/lib/Driver/ToolChains/Flang.cpp
index 6168b42dc78292..97326c2ed13f3c 100644
--- a/clang/lib/Driver/ToolChains/Flang.cpp
+++ b/clang/lib/Driver/ToolChains/Flang.cpp
@@ -38,8 +38,6 @@ void Flang::addFortranDialectOptions(const ArgList &Args,
Args.addAllArgs(CmdArgs, {options::OPT_ffixed_form,
options::OPT_ffree_form,
options::OPT_ffixed_line_length_EQ,
- options::OPT_fopenmp,
- options::OPT_fopenmp_version_EQ,
options::OPT_fopenacc,
options::OPT_finput_charset_EQ,
options::OPT_fimplicit_none,
@@ -764,6 +762,32 @@ void Flang::ConstructJob(Compilation &C, const JobAction &JA,
// Add other compile options
addOtherOptions(Args, CmdArgs);
+ // Forward flags for OpenMP. We don't do this if the current action is an
+ // device offloading action other than OpenMP.
+ if (Args.hasFlag(options::OPT_fopenmp, options::OPT_fopenmp_EQ,
+ options::OPT_fno_openmp, false) &&
+ (JA.isDeviceOffloading(Action::OFK_None) ||
+ JA.isDeviceOffloading(Action::OFK_OpenMP))) {
+ switch (D.getOpenMPRuntime(Args)) {
+ case Driver::OMPRT_OMP:
+ case Driver::OMPRT_IOMP5:
+ // Clang can generate useful OpenMP code for these two runtime libraries.
+ CmdArgs.push_back("-fopenmp");
+ Args.AddAllArgs(CmdArgs, options::OPT_fopenmp_version_EQ);
+
+ // FIXME: Clang supports a whole bunch more flags here.
+ break;
+ default:
+ // By default, if Clang doesn't know how to generate useful OpenMP code
+ // for a specific runtime library, we just don't pass the '-fopenmp' flag
+ // down to the actual compilation.
+ // FIXME: It would be better to have a mode which *only* omits IR
+ // generation based on the OpenMP support so that we get consistent
+ // semantic analysis, etc.
+ break;
+ }
+ }
+
// Offloading related options
addOffloadOptions(C, Inputs, JA, Args, CmdArgs);
diff --git a/flang/test/Driver/fopenmp.f90 b/flang/test/Driver/fopenmp.f90
new file mode 100644
index 00000000000000..068134a97a19d0
--- /dev/null
+++ b/flang/test/Driver/fopenmp.f90
@@ -0,0 +1,60 @@
+! RUN: %flang -target x86_64-linux-gnu -fopenmp=libomp -c %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-FC1-OPENMP
+! RUN: %flang -target x86_64-linux-gnu -fopenmp=libgomp -c %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-FC1-NO-OPENMP
+! RUN: %flang -target x86_64-linux-gnu -fopenmp=libiomp5 -c %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-FC1-OPENMP
+! RUN: %flang -target x86_64-apple-darwin -fopenmp=libomp -c %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-FC1-OPENMP
+! RUN: %flang -target x86_64-apple-darwin -fopenmp=libgomp -c %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-FC1-NO-OPENMP
+! RUN: %flang -target x86_64-apple-darwin -fopenmp=libiomp5 -c %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-FC1-OPENMP
+! RUN: %flang -target x86_64-freebsd -fopenmp=libomp -c %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-FC1-OPENMP
+! RUN: %flang -target x86_64-freebsd -fopenmp=libgomp -c %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-FC1-NO-OPENMP
+! RUN: %flang -target x86_64-freebsd -fopenmp=libiomp5 -c %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-FC1-OPENMP
+! RUN: %flang -target x86_64-windows-gnu -fopenmp=libomp -c %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-FC1-OPENMP
+! RUN: %flang -target x86_64-windows-gnu -fopenmp=libgomp -c %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-FC1-NO-OPENMP
+! RUN: %flang -target x86_64-windows-gnu -fopenmp=libiomp5 -c %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-FC1-OPENMP
+
+! CHECK-FC1-OPENMP: "-fc1"
+! CHECK-FC1-OPENMP: "-fopenmp"
+!
+! CHECK-FC1-NO-OPENMP: "-fc1"
+! CHECK-FC1-NO-OPENMP-NOT: "-fopenmp"
+!
+! RUN: %flang -target x86_64-linux-gnu -fopenmp=libomp %s -o %t -### 2>&1 | FileCheck %s --check-prefix=CHECK-LD-OMP
+! RUN: %flang -target x86_64-linux-gnu -fopenmp=libgomp %s -o %t -### 2>&1 | FileCheck %s --check-prefix=CHECK-LD-GOMP --check-prefix=CHECK-LD-GOMP-RT
+! RUN: %flang -target x86_64-linux-gnu -fopenmp=libiomp5 %s -o %t -### 2>&1 | FileCheck %s --check-prefix=CHECK-LD-IOMP5
+!
+! RUN: %flang -target x86_64-darwin -fopenmp=libomp %s -o %t -### 2>&1 | FileCheck %s --check-prefix=CHECK-LD-OMP
+! RUN: %flang -target x86_64-darwin -fopenmp=libgomp %s -o %t -### 2>&1 | FileCheck %s --check-prefix=CHECK-LD-GOMP --check-prefix=CHECK-LD-GOMP-NO-RT
+! RUN: %flang -target x86_64-darwin -fopenmp=libiomp5 %s -o %t -### 2>&1 | FileCheck %s --check-prefix=CHECK-LD-IOMP5
+!
+! RUN: %flang -target x86_64-freebsd -fopenmp=libomp %s -o %t -### 2>&1 | FileCheck %s --check-prefix=CHECK-LD-OMP
+! RUN: %flang -target x86_64-freebsd -fopenmp=libgomp %s -o %t -### 2>&1 | FileCheck %s --check-prefix=CHECK-LD-GOMP --check-prefix=CHECK-LD-GOMP-NO-RT
+! RUN: %flang -target x86_64-freebsd -fopenmp=libiomp5 %s -o %t -### 2>&1 | FileCheck %s --check-prefix=CHECK-LD-IOMP5
+!
+! RUN: %flang -target x86_64-windows-gnu -fopenmp=libomp %s -o %t -### 2>&1 | FileCheck %s --check-prefix=CHECK-LD-OMP
+! RUN: %flang -target x86_64-windows-gnu -fopenmp=libgomp %s -o %t -### 2>&1 | FileCheck %s --check-prefix=CHECK-LD-GOMP --check-prefix=CHECK-LD-GOMP-NO-RT
+! RUN: %flang -target x86_64-windows-gnu -fopenmp=libiomp5 %s -o %t -### 2>&1 | FileCheck %s --check-prefix=CHECK-LD-IOMP5MD
+!
+! CHECK-LD-OMP: "{{.*}}ld{{(.exe)?}}"
+! CHECK-LD-OMP: "-lomp"
+!
+! CHECK-LD-GOMP: "{{.*}}ld{{(.exe)?}}"
+! CHECK-LD-GOMP: "-lgomp"
+! CHECK-LD-GOMP-RT: "-lrt"
+! CHECK-LD-GOMP-NO-RT-NOT: "-lrt"
+!
+! CHECK-LD-IOMP5: "{{.*}}ld{{(.exe)?}}"
+! CHECK-LD-IOMP5: "-liomp5"
+!
+! CHECK-LD-IOMP5MD: "{{.*}}ld{{(.exe)?}}"
+! CHECK-LD-IOMP5MD: "-liomp5md"
+!
+! We'd like to check that the default is sane, but until we have the ability
+! to *always* semantically analyze OpenMP without always generating runtime
+! calls (in the event of an unsupported runtime), we don't have a good way to
+! test the CC1 invocation. Instead, just ensure we do eventually link *some*
+! OpenMP runtime.
+!
+! CHECK-LD-ANY: "{{.*}}ld{{(.exe)?}}"
+! CHECK-LD-ANY: "-l{{(omp|gomp|iomp5)}}"
+!
+! CHECK-LD-ANYMD: "{{.*}}ld{{(.exe)?}}"
+! CHECK-LD-ANYMD: "-l{{(omp|gomp|iomp5md)}}"
diff --git a/flang/test/Driver/omp-driver-offload.f90 b/flang/test/Driver/omp-driver-offload.f90
index 9b62699030c68f..7e9a73627cd757 100644
--- a/flang/test/Driver/omp-driver-offload.f90
+++ b/flang/test/Driver/omp-driver-offload.f90
@@ -57,9 +57,14 @@
! RUN: --target=aarch64-unknown-linux-gnu \
! RUN: | FileCheck %s --check-prefix=OPENMP-OFFLOAD-ARGS
! OPENMP-OFFLOAD-ARGS: "{{[^"]*}}flang-new" "-fc1" "-triple" "aarch64-unknown-linux-gnu" {{.*}} "-fopenmp" {{.*}}.f90"
-! OPENMP-OFFLOAD-ARGS-NEXT: "{{[^"]*}}flang-new" "-fc1" "-triple" "amdgcn-amd-amdhsa" {{.*}} "-fopenmp" {{.*}} "-fopenmp-host-ir-file-path" "{{.*}}.bc" "-fopenmp-is-target-device" {{.*}}.f90"
+! OPENMP-OFFLOAD-ARGS-NEXT: "{{[^"]*}}flang-new" "-fc1" "-triple" "amdgcn-amd-amdhsa"
+! OPENMP-OFFLOAD-ARGS-SAME: "-fopenmp"
+! OPENMP-OFFLOAD-ARGS-SAME: "-fopenmp-host-ir-file-path" "{{.*}}.bc" "-fopenmp-is-target-device"
+! OPENMP-OFFLOAD-ARGS-SAME: {{.*}}.f90"
! OPENMP-OFFLOAD-ARGS: "{{[^"]*}}clang-offload-packager{{.*}}" {{.*}} "--image=file={{.*}}.bc,triple=amdgcn-amd-amdhsa,arch=gfx90a,kind=openmp"
-! OPENMP-OFFLOAD-ARGS-NEXT: "{{[^"]*}}flang-new" "-fc1" "-triple" "aarch64-unknown-linux-gnu" {{.*}} "-fopenmp" {{.*}} "-fembed-offload-object={{.*}}.out" {{.*}}.bc"
+! OPENMP-OFFLOAD-ARGS-NEXT: "{{[^"]*}}flang-new" "-fc1" "-triple" "aarch64-unknown-linux-gnu"
+! OPENMP-OFFLOAD-ARGS-SAME: "-fopenmp"
+! OPENMP-OFFLOAD-ARGS-SAME: "-fembed-offload-object={{.*}}.out" {{.*}}.bc"
! Test -fopenmp with offload for RTL Flag Options
! RUN: %flang -### %s -o %t 2>&1 \
|
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
tblah
reviewed
Mar 27, 2024
| // Forward flags for OpenMP. We don't do this if the current action is an | ||
| // device offloading action other than OpenMP. | ||
| if (Args.hasFlag(options::OPT_fopenmp, options::OPT_fopenmp_EQ, | ||
| options::OPT_fno_openmp, false) && |
There was a problem hiding this comment.
I don't think -fno-openmp is visible in the flang driver. Maybe it should be.
|
|
||
| // FIXME: Clang supports a whole bunch more flags here. | ||
| break; | ||
| default: |
There was a problem hiding this comment.
Shouldn't we produce some sort of error in this case?
There was a problem hiding this comment.
To clarify, clang::Driver::getOpenMPRuntime does output an error (which we should test for), but currently we appear not to support libgomp, which clang does and so an error would not be produced in this case
There was a problem hiding this comment.
This is what Clang does in this place. I'm not sure I understand why.
There was a problem hiding this comment.
So, it definitely is done this way on purpose - see clang/test/Driver/fopenmp.c (only "interesting" lines)
// RUN: %clang -target x86_64-linux-gnu -fopenmp=libgomp -c %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-CC1-NO-OPENMP
...
// CHECK-CC1-OPENMP: "-cc1"
// CHECK-CC1-OPENMP: "-fopenmp"
//
// CHECK-CC1-NO-OPENMP: "-cc1"
// CHECK-CC1-NO-OPENMP-NOT: "-fopenmp"
We can certainly add a warning or error, but Clang doesn't... :)
There was a problem hiding this comment.
Clang might work differently internally. If we don't pass -fopenmp to the flang frontend driver, the parser will treat the openmp directives as comments. Which in many programs won't lead to any compilation errors but the program could produce different results (e.g. reductions won't happen)
|
|
||
| // FIXME: Clang supports a whole bunch more flags here. | ||
| break; | ||
| default: |
There was a problem hiding this comment.
Was leaving out OMPRT_GOMP deliberate? If so, please could you comment here or in the commit message explaining why and update the test not to be checking for libgomp.
There was a problem hiding this comment.
Yes, behaving the same way as clang here.
There was a problem hiding this comment.
If OMPRT_GOMP doesn't need -fopenmp for some reason then I don't think we understand how it works. Maybe we should prevent its use with flang for now?
llvmbot
added
the
clang:frontend
tblah
added a commit
to tblah/llvm-project
that referenced
this pull request
May 20, 2024
I believe these were forgotten when copying the clang in llvm#86816. This was flagged because the CHECK lines for CHECK-LD-ANY* had no associated RUN line. See llvm#92387 (comment)
tblah
added a commit
that referenced
this pull request
May 22, 2024
I believe these were forgotten when copying the clang in #86816. This was flagged because the CHECK lines for CHECK-LD-ANY* had no associated RUN line. See #92387 (comment)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This enables the -fopenmp= option to the set of options supported by flang.
The generated arguments for the FC1 compilation will appear in a slightly different order, so one test had to be updated to be less sensitive to order of the arguments.