[PATCH] D30087: [Driver] Unify linking of OpenMP runtime. NFCI.

[Jonas Hahnfeld via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL300689: [Driver] Unify linking of OpenMP runtime. NFCI. 
(authored by Hahnfeld).

Changed prior to commit:
  https://reviews.llvm.org/D30087?vs=94372=95733#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D30087

Files:
  cfe/trunk/lib/Driver/ToolChains/CommonArgs.cpp
  cfe/trunk/lib/Driver/ToolChains/CommonArgs.h
  cfe/trunk/lib/Driver/ToolChains/Gnu.cpp
  cfe/trunk/test/Driver/fopenmp.c

Index: cfe/trunk/lib/Driver/ToolChains/Gnu.cpp
===
--- cfe/trunk/lib/Driver/ToolChains/Gnu.cpp
+++ cfe/trunk/lib/Driver/ToolChains/Gnu.cpp
@@ -586,37 +586,15 @@
   bool WantPthread = Args.hasArg(options::OPT_pthread) ||
  Args.hasArg(options::OPT_pthreads);
 
-  if (Args.hasFlag(options::OPT_fopenmp, options::OPT_fopenmp_EQ,
-   options::OPT_fno_openmp, false)) {
+  // FIXME: Only pass GompNeedsRT = true for platforms with libgomp that
+  // require librt. Most modern Linux platforms do, but some may not.
+  if (addOpenMPRuntime(CmdArgs, ToolChain, Args,
+   JA.isHostOffloading(Action::OFK_OpenMP),
+   /* GompNeedsRT= */ true))
 // OpenMP runtimes implies pthreads when using the GNU toolchain.
 // FIXME: Does this really make sense for all GNU toolchains?
 WantPthread = true;
 
-// Also link the particular OpenMP runtimes.
-switch (ToolChain.getDriver().getOpenMPRuntime(Args)) {
-case Driver::OMPRT_OMP:
-  CmdArgs.push_back("-lomp");
-  break;
-case Driver::OMPRT_GOMP:
-  CmdArgs.push_back("-lgomp");
-
-  // FIXME: Exclude this for platforms with libgomp that don't require
-  // librt. Most modern Linux platforms require it, but some may not.
-  CmdArgs.push_back("-lrt");
-  break;
-case Driver::OMPRT_IOMP5:
-  CmdArgs.push_back("-liomp5");
-  break;
-case Driver::OMPRT_Unknown:
-  // Already diagnosed.
-  break;
-}
-if (JA.isHostOffloading(Action::OFK_OpenMP))
-  CmdArgs.push_back("-lomptarget");
-
-addArchSpecificRPath(ToolChain, Args, CmdArgs);
-  }
-
   AddRunTimeLibs(ToolChain, D, CmdArgs, Args);
 
   if (WantPthread && !isAndroid)
Index: cfe/trunk/lib/Driver/ToolChains/CommonArgs.cpp
===
--- cfe/trunk/lib/Driver/ToolChains/CommonArgs.cpp
+++ cfe/trunk/lib/Driver/ToolChains/CommonArgs.cpp
@@ -426,28 +426,37 @@
   }
 }
 
-void tools::addOpenMPRuntime(ArgStringList , const ToolChain ,
- const ArgList ) {
+bool tools::addOpenMPRuntime(ArgStringList , const ToolChain ,
+ const ArgList , bool IsOffloadingHost,
+ bool GompNeedsRT) {
   if (!Args.hasFlag(options::OPT_fopenmp, options::OPT_fopenmp_EQ,
 options::OPT_fno_openmp, false))
-return;
+return false;
 
   switch (TC.getDriver().getOpenMPRuntime(Args)) {
   case Driver::OMPRT_OMP:
 CmdArgs.push_back("-lomp");
 break;
   case Driver::OMPRT_GOMP:
 CmdArgs.push_back("-lgomp");
+
+if (GompNeedsRT)
+  CmdArgs.push_back("-lrt");
 break;
   case Driver::OMPRT_IOMP5:
 CmdArgs.push_back("-liomp5");
 break;
   case Driver::OMPRT_Unknown:
 // Already diagnosed.
-break;
+return false;
   }
 
+  if (IsOffloadingHost)
+CmdArgs.push_back("-lomptarget");
+
   addArchSpecificRPath(TC, Args, CmdArgs);
+
+  return true;
 }
 
 static void addSanitizerRuntime(const ToolChain , const ArgList ,
Index: cfe/trunk/lib/Driver/ToolChains/CommonArgs.h
===
--- cfe/trunk/lib/Driver/ToolChains/CommonArgs.h
+++ cfe/trunk/lib/Driver/ToolChains/CommonArgs.h
@@ -59,8 +59,10 @@
 
 void addArchSpecificRPath(const ToolChain , const llvm::opt::ArgList ,
   llvm::opt::ArgStringList );
-void addOpenMPRuntime(llvm::opt::ArgStringList , const ToolChain ,
-  const llvm::opt::ArgList );
+/// Returns true, if an OpenMP runtime has been added.
+bool addOpenMPRuntime(llvm::opt::ArgStringList , const ToolChain ,
+  const llvm::opt::ArgList ,
+  bool IsOffloadingHost = false, bool GompNeedsRT = false);
 
 llvm::opt::Arg *getLastProfileUseArg(const llvm::opt::ArgList );
 llvm::opt::Arg *getLastProfileSampleUseArg(const llvm::opt::ArgList );
Index: cfe/trunk/test/Driver/fopenmp.c
===
--- cfe/trunk/test/Driver/fopenmp.c
+++ cfe/trunk/test/Driver/fopenmp.c
@@ -18,29 +18,33 @@
 // CHECK-CC1-NO-OPENMP-NOT: "-fopenmp"
 //
 // RUN: %clang -target x86_64-linux-gnu -fopenmp=libomp %s -o %t -### 2>&1 | FileCheck %s 

[PATCH] D30087: [Driver] Unify linking of OpenMP runtime. NFCI.

[Alexey Bataev via Phabricator via cfe-commits]

ABataev accepted this revision.
ABataev added a comment.
This revision is now accepted and ready to land.

LG


https://reviews.llvm.org/D30087



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D30087: [Driver] Unify linking of OpenMP runtime. NFCI.

[Jonas Hahnfeld via Phabricator via cfe-commits]

Hahnfeld added a comment.

Ping


https://reviews.llvm.org/D30087



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D30087: [Driver] Unify linking of OpenMP runtime. NFCI.

[Jonas Hahnfeld via Phabricator via cfe-commits]

Hahnfeld updated this revision to Diff 94372.
Hahnfeld marked 2 inline comments as done.
Hahnfeld retitled this revision from "[Driver] Unify linking of OpenMP runtime" 
to "[Driver] Unify linking of OpenMP runtime. NFCI.".
Hahnfeld edited the summary of this revision.

https://reviews.llvm.org/D30087

Files:
  lib/Driver/ToolChains/CommonArgs.cpp
  lib/Driver/ToolChains/CommonArgs.h
  lib/Driver/ToolChains/Gnu.cpp
  test/Driver/fopenmp.c

Index: test/Driver/fopenmp.c
===
--- test/Driver/fopenmp.c
+++ test/Driver/fopenmp.c
@@ -18,29 +18,33 @@
 // CHECK-CC1-NO-OPENMP-NOT: "-fopenmp"
 //
 // RUN: %clang -target x86_64-linux-gnu -fopenmp=libomp %s -o %t -### 2>&1 | FileCheck %s --check-prefix=CHECK-LD-OMP
-// RUN: %clang -target x86_64-linux-gnu -fopenmp=libgomp %s -o %t -### 2>&1 | FileCheck %s --check-prefix=CHECK-LD-GOMP
+// RUN: %clang -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: %clang -target x86_64-linux-gnu -fopenmp=libiomp5 %s -o %t -### 2>&1 | FileCheck %s --check-prefix=CHECK-LD-IOMP5
 //
 // RUN: %clang -nostdlib -target x86_64-linux-gnu -fopenmp=libomp %s -o %t -### 2>&1 | FileCheck %s --check-prefix=CHECK-NO-OMP
 // RUN: %clang -nostdlib -target x86_64-linux-gnu -fopenmp=libgomp %s -o %t -### 2>&1 | FileCheck %s --check-prefix=CHECK-NO-GOMP
 // RUN: %clang -nostdlib -target x86_64-linux-gnu -fopenmp=libiomp5 %s -o %t -### 2>&1 | FileCheck %s --check-prefix=CHECK-NO-IOMP5
 //
 // RUN: %clang -target x86_64-darwin -fopenmp=libomp %s -o %t -### 2>&1 | FileCheck %s --check-prefix=CHECK-LD-OMP
-// RUN: %clang -target x86_64-darwin -fopenmp=libgomp %s -o %t -### 2>&1 | FileCheck %s --check-prefix=CHECK-LD-GOMP
+// RUN: %clang -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: %clang -target x86_64-darwin -fopenmp=libiomp5 %s -o %t -### 2>&1 | FileCheck %s --check-prefix=CHECK-LD-IOMP5
 //
 // RUN: %clang -nostdlib -target x86_64-darwin -fopenmp=libomp %s -o %t -### 2>&1 | FileCheck %s --check-prefix=CHECK-NO-OMP
 // RUN: %clang -nostdlib -target x86_64-darwin -fopenmp=libgomp %s -o %t -### 2>&1 | FileCheck %s --check-prefix=CHECK-NO-GOMP
 // RUN: %clang -nostdlib -target x86_64-darwin -fopenmp=libiomp5 %s -o %t -### 2>&1 | FileCheck %s --check-prefix=CHECK-NO-IOMP5
 //
-// RUN: %clang -target x86_64-netbsd -fopenmp=libomp %s -o %t -### 2>&1 | FileCheck %s --check-prefix=CHECK-LD-OMP
-// RUN: %clang -target x86_64-netbsd -fopenmp=libgomp %s -o %t -### 2>&1 | FileCheck %s --check-prefix=CHECK-LD-GOMP
-// RUN: %clang -target x86_64-netbsd -fopenmp=libiomp5 %s -o %t -### 2>&1 | FileCheck %s --check-prefix=CHECK-LD-IOMP5
+// RUN: %clang -target x86_64-freebsd -fopenmp=libomp %s -o %t -### 2>&1 | FileCheck %s --check-prefix=CHECK-LD-OMP
+// RUN: %clang -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: %clang -target x86_64-freebsd -fopenmp=libiomp5 %s -o %t -### 2>&1 | FileCheck %s --check-prefix=CHECK-LD-IOMP5
 //
 // RUN: %clang -nostdlib -target x86_64-freebsd -fopenmp=libomp %s -o %t -### 2>&1 | FileCheck %s --check-prefix=CHECK-NO-OMP
 // RUN: %clang -nostdlib -target x86_64-freebsd -fopenmp=libgomp %s -o %t -### 2>&1 | FileCheck %s --check-prefix=CHECK-NO-GOMP
 // RUN: %clang -nostdlib -target x86_64-freebsd -fopenmp=libiomp5 %s -o %t -### 2>&1 | FileCheck %s --check-prefix=CHECK-NO-IOMP5
 //
+// RUN: %clang -target x86_64-netbsd -fopenmp=libomp %s -o %t -### 2>&1 | FileCheck %s --check-prefix=CHECK-LD-OMP
+// RUN: %clang -target x86_64-netbsd -fopenmp=libgomp %s -o %t -### 2>&1 | FileCheck %s --check-prefix=CHECK-LD-GOMP --check-prefix=CHECK-LD-GOMP-NO-RT
+// RUN: %clang -target x86_64-netbsd -fopenmp=libiomp5 %s -o %t -### 2>&1 | FileCheck %s --check-prefix=CHECK-LD-IOMP5
+//
 // RUN: %clang -nostdlib -target x86_64-netbsd -fopenmp=libomp %s -o %t -### 2>&1 | FileCheck %s --check-prefix=CHECK-NO-OMP
 // RUN: %clang -nostdlib -target x86_64-netbsd -fopenmp=libgomp %s -o %t -### 2>&1 | FileCheck %s --check-prefix=CHECK-NO-GOMP
 // RUN: %clang -nostdlib -target x86_64-netbsd -fopenmp=libiomp5 %s -o %t -### 2>&1 | FileCheck %s --check-prefix=CHECK-NO-IOMP5
@@ -50,6 +54,8 @@
 //
 // 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"
Index: lib/Driver/ToolChains/Gnu.cpp
===
--- lib/Driver/ToolChains/Gnu.cpp
+++ lib/Driver/ToolChains/Gnu.cpp
@@ -586,37 +586,15 @@
   bool WantPthread = Args.hasArg(options::OPT_pthread) ||
  Args.hasArg(options::OPT_pthreads);
 
-  if (Args.hasFlag(options::OPT_fopenmp, 

[PATCH] D30087: [Driver] Unify linking of OpenMP runtime. NFCI.

[Jonas Hahnfeld via Phabricator via cfe-commits]

Hahnfeld added inline comments.



Comment at: lib/Driver/ToolChains/CommonArgs.cpp:430
+bool tools::addOpenMPRuntime(ArgStringList , const ToolChain ,
+ const ArgList , const JobAction ,
+ bool GompNeedsRT) {

ABataev wrote:
> Do you really need to pass a reference to `JobAction` here or it is enough to 
> pass a bool value for `JA.isHostOffloading()`?
Good idea, this even allows this change to become fully NFC


https://reviews.llvm.org/D30087



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits