[OpenMP] Fix passing of -m arguments correctly

[Hahnfeld]

Summary

The recent fix in D38258 was wrong: getAuxTriple() only returns
non-null values for the CUDA toolchain. That is why the now added
test for PPC and X86 failed.

[hfinkel]

LGTM

include/clang/Driver/ToolChain.h

public:
   TranslateArgs(const llvm::opt::DerivedArgList &Args, StringRef BoundArch,
                 Action::OffloadKind DeviceOffloadKind) const {
     return nullptr;
   }
 
   /// TranslateOpenMPTargetArgs - Create a new derived argument list for
   /// that contains the OpenMP target specific flags passed via
   /// -Xopenmp-target -opt=val OR -Xopenmp-target=<triple> -opt=val
-  /// Translation occurs only when the \p DeviceOffloadKind is specified.
-  ///
-  /// \param DeviceOffloadKind - The device offload kind used for the
-  /// translation.
   virtual llvm::opt::DerivedArgList *TranslateOpenMPTargetArgs(
-      const llvm::opt::DerivedArgList &Args,
-      Action::OffloadKind DeviceOffloadKind,
-      SmallVector<llvm::opt::Arg *, 4> &AllocatedArgs) const;
+      const llvm::opt::DerivedArgList &Args, bool SameTripleAsHost,
+      SmallVectorImpl<llvm::opt::Arg *> &AllocatedArgs) const;
 
   /// Choose a tool to use to handle the action \p JA.
   ///
   /// This can be overridden when a particular ToolChain needs to use
   /// a compiler other than Clang.
   virtual Tool *SelectTool(const JobAction &JA) const;
 
   // Helper methods

lib/Driver/Compilation.cpp

 Compilation::getArgsForToolChain(const ToolChain *TC, StringRef BoundArch,
                                  Action::OffloadKind DeviceOffloadKind) {
   if (!TC)
     TC = &DefaultToolChain;
 
   DerivedArgList *&Entry = TCArgs[{TC, BoundArch, DeviceOffloadKind}];
   if (!Entry) {
     SmallVector<Arg *, 4> AllocatedArgs;
+    DerivedArgList *OpenMPArgs = nullptr;
     // Translate OpenMP toolchain arguments provided via the -Xopenmp-target flags.
-    DerivedArgList *OpenMPArgs = TC->TranslateOpenMPTargetArgs(
-        *TranslatedArgs, DeviceOffloadKind, AllocatedArgs);
+    if (DeviceOffloadKind == Action::OFK_OpenMP) {
+      const ToolChain *HostTC = getSingleOffloadToolChain<Action::OFK_Host>();
+      bool SameTripleAsHost = (TC->getTriple() == HostTC->getTriple());
+      OpenMPArgs = TC->TranslateOpenMPTargetArgs(
+          *TranslatedArgs, SameTripleAsHost, AllocatedArgs);
+    }
+
     if (!OpenMPArgs) {
       Entry = TC->TranslateArgs(*TranslatedArgs, BoundArch, DeviceOffloadKind);
       if (!Entry)
         Entry = TranslatedArgs;
     } else {
       Entry = TC->TranslateArgs(*OpenMPArgs, BoundArch, DeviceOffloadKind);
       if (!Entry)
         Entry = OpenMPArgs;

lib/Driver/ToolChain.cpp

if (StringRef(MSCVersion->getValue()).getAsInteger(10, Version)) {
       return separateMSVCFullVersion(Version);
     }
   }
 
   return VersionTuple();
 }
 
 llvm::opt::DerivedArgList *ToolChain::TranslateOpenMPTargetArgs(
-    const llvm::opt::DerivedArgList &Args,
-    Action::OffloadKind DeviceOffloadKind,
-    SmallVector<llvm::opt::Arg *, 4> &AllocatedArgs) const {
-  if (DeviceOffloadKind == Action::OFK_OpenMP) {
-    DerivedArgList *DAL = new DerivedArgList(Args.getBaseArgs());
-    const OptTable &Opts = getDriver().getOpts();
-    bool Modified = false;
+    const llvm::opt::DerivedArgList &Args, bool SameTripleAsHost,
+    SmallVectorImpl<llvm::opt::Arg *> &AllocatedArgs) const {
 
-    // Handle -Xopenmp-target flags
-    for (Arg *A : Args) {
-      // Exclude flags which may only apply to the host toolchain.
-      // Do not exclude flags when the host triple (AuxTriple)
-      // matches the current toolchain triple. If it is not present
-      // at all, target and host share a toolchain.
-      if (A->getOption().matches(options::OPT_m_Group)) {
-        if (!getAuxTriple() || getAuxTriple()->str() == getTriple().str())
-          DAL->append(A);
-        else
-          Modified = true;
-        continue;
-      }
+      if (SameTripleAsHost)
 
-      unsigned Index;
-      unsigned Prev;
-      bool XOpenMPTargetNoTriple = A->getOption().matches(
-          options::OPT_Xopenmp_target);
+    bool XOpenMPTargetNoTriple =
+        A->getOption().matches(options::OPT_Xopenmp_target);
 
-      if (A->getOption().matches(options::OPT_Xopenmp_target_EQ)) {
-        // Passing device args: -Xopenmp-target=<triple> -opt=val.
-        if (A->getValue(0) == getTripleString())
-          Index = Args.getBaseArgs().MakeIndex(A->getValue(1));
-        else
-          continue;
-      } else if (XOpenMPTargetNoTriple) {
-        // Passing device args: -Xopenmp-target -opt=val.
-        Index = Args.getBaseArgs().MakeIndex(A->getValue(0));
-      } else {
-        DAL->append(A);
-        continue;
-      }
 
-      // Parse the argument to -Xopenmp-target.
-      Prev = Index;
-      std::unique_ptr<Arg> XOpenMPTargetArg(Opts.ParseOneArg(Args, Index));
-      if (!XOpenMPTargetArg || Index > Prev + 1) {
-        getDriver().Diag(diag::err_drv_invalid_Xopenmp_target_with_args)
-            << A->getAsString(Args);
-        continue;
-      }
-      if (XOpenMPTargetNoTriple && XOpenMPTargetArg &&
-          Args.getAllArgValues(
-              options::OPT_fopenmp_targets_EQ).size() != 1) {
-        getDriver().Diag(diag::err_drv_Xopenmp_target_missing_triple);
-        continue;
-      }
-      XOpenMPTargetArg->setBaseArg(A);
-      A = XOpenMPTargetArg.release();
-      AllocatedArgs.push_back(A);
-      DAL->append(A);
-      Modified = true;
-    }
+        Args.getAllArgValues(options::OPT_fopenmp_targets_EQ).size() != 1) {
 
-    if (Modified) {
-      return DAL;
-    } else {
-      delete DAL;
-    }
-  }
+  if (Modified)
 
+  delete DAL;
   return nullptr;
 }

test/Driver/openmp-offload.c

 /// Check -march=pwr7 is NOT passed to nvptx64-nvidia-cuda.
 // RUN:    %clang -### -no-canonical-prefixes -fopenmp=libomp -fopenmp-targets=nvptx64-nvidia-cuda -march=pwr7 %s 2>&1 \
 // RUN:    | FileCheck -check-prefix=CHK-FOPENMP-MARCH-TO-GPU %s
 
 // CHK-FOPENMP-MARCH-TO-GPU-NOT: clang{{.*}} "-target-cpu" "pwr7" {{.*}}"-fopenmp-is-device"
 
 /// ###########################################################################
 
+/// Check -march=pwr7 is NOT passed to x86_64-unknown-linux-gnu.
+// RUN:    %clang -### -no-canonical-prefixes -fopenmp=libomp -fopenmp-targets=x86_64-unknown-linux-gnu -march=pwr7 %s 2>&1 \
+// RUN:    | FileCheck -check-prefix=CHK-FOPENMP-MARCH-TO-X86 %s
+
+// CHK-FOPENMP-MARCH-TO-X86-NOT: clang{{.*}} "-target-cpu" "pwr7" {{.*}}"-fopenmp-is-device"
+
+/// ###########################################################################
+
 /// Check -Xopenmp-target triggers error when multiple triples are used.
 // RUN:   %clang -### -no-canonical-prefixes -fopenmp=libomp -fopenmp-targets=powerpc64le-ibm-linux-gnu,powerpc64le-unknown-linux-gnu -Xopenmp-target -mcpu=pwr8 %s 2>&1 \
 // RUN:   | FileCheck -check-prefix=CHK-FOPENMP-TARGET-AMBIGUOUS-ERROR %s
 
 // CHK-FOPENMP-TARGET-AMBIGUOUS-ERROR: clang{{.*}} error: cannot deduce implicit triple value for -Xopenmp-target, specify triple using -Xopenmp-target=<triple>
 
 /// ###########################################################################