[Driver] Unify linking of OpenMP runtime. NFCI.

[Hahnfeld]

Summary

While at it, extend test for FreeBSD and check for -lrt iff on Linux.

[pirama]

lib/Driver/Tools.cpp

8683

addOpenMPRuntime would add -lomptarget even for this call if JA satisfies the newly added check. That's a deviation from exisiting behavior. Am I missing some invariant here where JobActions along this path don't have an offloading action?

[Hahnfeld]

lib/Driver/Tools.cpp

8683

Right, as I noted in the summary: This enables libomptarget for Darwin, FreeBSD and NetBSD

I don't see a problem with that, it's rather an inconsistency in the current code.

[Hahnfeld]

rebase

[Hahnfeld]

Ping

[hfinkel]

lib/Driver/Tools.cpp

10334–10337

Now that you've moved the comment, it is not clear what "this" means here. Please reword this to say that you should only pass true for GompNeedsRT on platforms that really need it.

[Hahnfeld]

Reword comment

[Hahnfeld]

Rebase for recent refactoring and ping.

[Hahnfeld]

Another ping

[Hahnfeld]

Rebase and ping.

[Hahnfeld]

Ping!

[ABataev]

lib/Driver/ToolChains/CommonArgs.cpp

430 ↗

(On Diff #93213)

Do you really need to pass a reference to JobAction here or it is enough to pass a bool value for JA.isHostOffloading()?

443–445 ↗

(On Diff #93213)

Remove braces here

[Hahnfeld]

lib/Driver/ToolChains/CommonArgs.cpp

430 ↗

(On Diff #93213)

Good idea, this even allows this change to become fully NFC

[Hahnfeld]

Ping

[ABataev]

LG

lib/Driver/Tools.cpp

 // This adds the static libclang_rt.builtins-arch.a directly to the command line
 // FIXME: Make sure we can also emit shared objects if they're requested
 // and available, check for possible errors, etc.
 static void addClangRT(const ToolChain &TC, const ArgList &Args,
                        ArgStringList &CmdArgs) {
   CmdArgs.push_back(TC.getCompilerRTArgString(Args, "builtins"));
 }
 
-static void addOpenMPRuntime(ArgStringList &CmdArgs, const ToolChain &TC,
-                              const ArgList &Args) {
+// Returns true, if an OpenMP runtime has been added.
+static bool addOpenMPRuntime(ArgStringList &CmdArgs, const ToolChain &TC,
+                             const ArgList &Args, const JobAction &JA,
+                             bool GompNeedsRT = false) {
   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 (JA.isHostOffloading(Action::OFK_OpenMP))
+    CmdArgs.push_back("-lomptarget");
+
+  return true;
 }
 
 static void addSanitizerRuntime(const ToolChain &TC, const ArgList &Args,
                                 ArgStringList &CmdArgs, StringRef Sanitizer,
                                 bool IsShared, bool IsWhole) {
   // Wrap any static runtimes that must be forced into executable in
   // whole-archive.
   if (IsWhole) CmdArgs.push_back("-whole-archive");
if (!II.isFilename()) {
 
       continue;
     }
 
     InputFileList.push_back(II.getFilename());
   }
 
   if (!Args.hasArg(options::OPT_nostdlib, options::OPT_nodefaultlibs))
-    addOpenMPRuntime(CmdArgs, getToolChain(), Args);
+    addOpenMPRuntime(CmdArgs, getToolChain(), Args, JA);
 
   if (isObjCRuntimeLinked(Args) &&
       !Args.hasArg(options::OPT_nostdlib, options::OPT_nodefaultlibs)) {
     // We use arclite library for both ARC and subscripting support.
     getMachOToolChain().AddLinkARCArgs(Args, CmdArgs);
 
     CmdArgs.push_back("-framework");
     CmdArgs.push_back("Foundation");
void freebsd::Linker::ConstructJob(Compilation &C, const JobAction &JA,
 
   if (D.isUsingLTO())
     AddGoldPlugin(ToolChain, Args, CmdArgs, D.getLTOMode() == LTOK_Thin, D);
 
   bool NeedsSanitizerDeps = addSanitizerRuntimes(ToolChain, Args, CmdArgs);
   AddLinkerInputs(ToolChain, Inputs, Args, CmdArgs, JA);
 
   if (!Args.hasArg(options::OPT_nostdlib, options::OPT_nodefaultlibs)) {
-    addOpenMPRuntime(CmdArgs, ToolChain, Args);
+    addOpenMPRuntime(CmdArgs, ToolChain, Args, JA);
     if (D.CCCIsCXX()) {
       ToolChain.AddCXXStdlibLibArgs(Args, CmdArgs);
       if (Args.hasArg(options::OPT_pg))
         CmdArgs.push_back("-lm_p");
       else
         CmdArgs.push_back("-lm");
     }
     if (NeedsSanitizerDeps)
case llvm::Triple::x86_64:
       useLibgcc = false;
       break;
     default:
       break;
     }
   }
 
   if (!Args.hasArg(options::OPT_nostdlib, options::OPT_nodefaultlibs)) {
-    addOpenMPRuntime(CmdArgs, getToolChain(), Args);
+    addOpenMPRuntime(CmdArgs, getToolChain(), Args, JA);
     if (D.CCCIsCXX()) {
       getToolChain().AddCXXStdlibLibArgs(Args, CmdArgs);
       CmdArgs.push_back("-lm");
     }
     if (Args.hasArg(options::OPT_pthread))
       CmdArgs.push_back("-lpthread");
     CmdArgs.push_back("-lc");
 
if (!Args.hasArg(options::OPT_nodefaultlibs)) {
         linkSanitizerRuntimeDeps(ToolChain, CmdArgs);
 
       if (NeedsXRayDeps)
         linkXRayRuntimeDeps(ToolChain, Args, CmdArgs);
 
       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: Exclude this for platforms with libgomp that don't require
+      // librt. Most modern Linux platforms require it, but some may not.
+      if (addOpenMPRuntime(CmdArgs, ToolChain, Args, JA,
+                           /* 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");
-      }
-
       AddRunTimeLibs(ToolChain, D, CmdArgs, Args);
 
       if (WantPthread && !isAndroid)
         CmdArgs.push_back("-lpthread");
 
       if (Args.hasArg(options::OPT_fsplit_stack))
         CmdArgs.push_back("--wrap=pthread_create");
 

test/Driver/fopenmp.c

 //
 // CHECK-CC1-OPENMP: "-cc1"
 // CHECK-CC1-OPENMP: "-fopenmp"
 //
 // CHECK-CC1-NO-OPENMP: "-cc1"
 // 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
 //
 // 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-NO-OMP: "{{.*}}ld{{(.exe)?}}"
 // CHECK-NO-OMP-NOT: "-lomp"
 //
 // CHECK-NO-GOMP: "{{.*}}ld{{(.exe)?}}"