[CUDA][HIP] Add -Xarch_device and -Xarch_host options

[yaxunl]

Summary

The argument after -Xarch_device will be added to the arguments for CUDA/HIP
device compilation and will be removed for host compilation.

The argument after -Xarch_host will be added to the arguments for CUDA/HIP
host compilation and will be removed for device compilation.

[tra]

Does it handle options with values? E.g. if I want to pass -mframe-pointer=none ? I vaguely recall the current -Xarch_* implementation had some limitations.
It may be worth adding a test for that.

[yaxunl]

Add a test for passing options with value

[yaxunl]

In D76520#1934341, @tra wrote:

Does it handle options with values? E.g. if I want to pass -mframe-pointer=none ? I vaguely recall the current -Xarch_* implementation had some limitations.
It may be worth adding a test for that.

-Xarch_ works with driver options having value, e.g. -fcf-protection=branch. I added a test for that.

-mframe-pointer=none is a cc1 option. That's why it cannot be passed by -Xarch_. If it is made a driver option it can be passed.

[tra]

In D76520#1936837, @yaxunl wrote:

-Xarch_ works with driver options having value, e.g. -fcf-protection=branch. I added a test for that.

-mframe-pointer=none is a cc1 option. That's why it cannot be passed by -Xarch_. If it is made a driver option it can be passed.

In other words, if we need to pass a cc1 option to a specific sub-compilation, we'll need an arch-specific -Xclang variant. Does -Xarch_foo -Xclang -Xarch_foo <foo-specific cc1 option> work with this patch? Considering that sub-compilations *are* cc1 invocations, passing cc1 options is something that would be very useful as not all cc1 options can be directly controlled from the driver level, but most driver options not directly related to the pipeline construction can be overridden by cc1 options.

[yaxunl]

In D76520#1937217, @tra wrote:

In D76520#1936837, @yaxunl wrote:

-Xarch_ works with driver options having value, e.g. -fcf-protection=branch. I added a test for that.

-mframe-pointer=none is a cc1 option. That's why it cannot be passed by -Xarch_. If it is made a driver option it can be passed.

In other words, if we need to pass a cc1 option to a specific sub-compilation, we'll need an arch-specific -Xclang variant. Does -Xarch_foo -Xclang -Xarch_foo <foo-specific cc1 option> work with this patch? Considering that sub-compilations *are* cc1 invocations, passing cc1 options is something that would be very useful as not all cc1 options can be directly controlled from the driver level, but most driver options not directly related to the pipeline construction can be overridden by cc1 options.

-Xarch_ does not work for passing -cc1 options in the beginning. This patch does not change that.

This requires some further changes about how the options after -Xarch_ are handled. I would suggest to do that in another patch.

[tra]

In D76520#1937294, @yaxunl wrote:

-Xarch_ does not work for passing -cc1 options in the beginning. This patch does not change that.

This requires some further changes about how the options after -Xarch_ are handled. I would suggest to do that in another patch.

I'm OK with a separate patch.
Meanwhile we should document that it only works for driver args. Perhaps add a TODO comment in the TranslateXarchArgs describing its limitations.

[yaxunl]

add TODO for fixing space separated arguments

clang/include/clang/Driver/Options.td

 def Wwrite_strings : Flag<["-"], "Wwrite-strings">, Group<W_Group>, Flags<[CC1Option, HelpHidden]>;
 def Wno_write_strings : Flag<["-"], "Wno-write-strings">, Group<W_Group>, Flags<[CC1Option, HelpHidden]>;
 def W_Joined : Joined<["-"], "W">, Group<W_Group>, Flags<[CC1Option, CoreOption]>,
   MetaVarName<"<warning>">, HelpText<"Enable the specified warning">;
 def Xanalyzer : Separate<["-"], "Xanalyzer">,
   HelpText<"Pass <arg> to the static analyzer">, MetaVarName<"<arg>">,
   Group<StaticAnalyzer_Group>;
 def Xarch__ : JoinedAndSeparate<["-"], "Xarch_">, Flags<[DriverOption]>;
+def Xarch_host : Separate<["-"], "Xarch_host">, Flags<[DriverOption]>,
+  HelpText<"Pass <arg> to the CUDA/HIP host compilation">, MetaVarName<"<arg>">;
+def Xarch_device : Separate<["-"], "Xarch_device">, Flags<[DriverOption]>,
+  HelpText<"Pass <arg> to the CUDA/HIP device compilation">, MetaVarName<"<arg>">;
 def Xassembler : Separate<["-"], "Xassembler">,
   HelpText<"Pass <arg> to the assembler">, MetaVarName<"<arg>">,
   Group<CompileOnly_Group>;
 def Xclang : Separate<["-"], "Xclang">,
   HelpText<"Pass <arg> to the clang compiler">, MetaVarName<"<arg>">,
   Flags<[DriverOption, CoreOption]>, Group<CompileOnly_Group>;
 def Xcuda_fatbinary : Separate<["-"], "Xcuda-fatbinary">,
   HelpText<"Pass <arg> to fatbinary invocation">, MetaVarName<"<arg>">;

clang/include/clang/Driver/ToolChain.h

public:
   /// 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
   virtual llvm::opt::DerivedArgList *TranslateOpenMPTargetArgs(
       const llvm::opt::DerivedArgList &Args, bool SameTripleAsHost,
       SmallVectorImpl<llvm::opt::Arg *> &AllocatedArgs) const;
 
   /// Append the argument following \p A to \p DAL assuming \p A is an Xarch
-  /// argument.
-  virtual void TranslateXarchArgs(const llvm::opt::DerivedArgList &Args,
-                                  llvm::opt::Arg *&A,
-                                  llvm::opt::DerivedArgList *DAL) const;
+  /// argument. If \p AllocatedArgs is null pointer, synthesized arguments are
+  /// added to \p DAL, otherwise they are appended to \p AllocatedArgs.
+  virtual void TranslateXarchArgs(
+      const llvm::opt::DerivedArgList &Args, llvm::opt::Arg *&A,
+      llvm::opt::DerivedArgList *DAL,
+      SmallVectorImpl<llvm::opt::Arg *> *AllocatedArgs = nullptr) const;
+
+  /// Translate -Xarch_ arguments. If there are no such arguments, return
+  /// a null pointer, otherwise return a DerivedArgList containing the
+  /// translated arguments.
+  virtual llvm::opt::DerivedArgList *
+  TranslateXarchArgs(const llvm::opt::DerivedArgList &Args, StringRef BoundArch,
+                     Action::OffloadKind DeviceOffloadKind,
+                     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

clang/lib/Driver/Compilation.cpp

if (!Entry) {
     // Translate OpenMP toolchain arguments provided via the -Xopenmp-target flags.
     if (DeviceOffloadKind == Action::OFK_OpenMP) {
       const ToolChain *HostTC = getSingleOffloadToolChain<Action::OFK_Host>();
       bool SameTripleAsHost = (TC->getTriple() == HostTC->getTriple());
       OpenMPArgs = TC->TranslateOpenMPTargetArgs(
           *TranslatedArgs, SameTripleAsHost, AllocatedArgs);
     }
 
+    DerivedArgList *NewDAL = nullptr;
     if (!OpenMPArgs) {
+      NewDAL = TC->TranslateXarchArgs(*TranslatedArgs, BoundArch,
+                                      DeviceOffloadKind, &AllocatedArgs);
+    } else {
+      NewDAL = TC->TranslateXarchArgs(*OpenMPArgs, BoundArch, DeviceOffloadKind,
+                                      &AllocatedArgs);
+      if (!NewDAL)
+        NewDAL = OpenMPArgs;
+      else
+        delete OpenMPArgs;
+    }
+
+    if (!NewDAL) {
       Entry = TC->TranslateArgs(*TranslatedArgs, BoundArch, DeviceOffloadKind);
       if (!Entry)
         Entry = TranslatedArgs;
     } else {
-      Entry = TC->TranslateArgs(*OpenMPArgs, BoundArch, DeviceOffloadKind);
+      Entry = TC->TranslateArgs(*NewDAL, BoundArch, DeviceOffloadKind);
       if (!Entry)
-        Entry = OpenMPArgs;
+        Entry = NewDAL;
       else
-        delete OpenMPArgs;
+        delete NewDAL;
     }
 
     // Add allocated arguments to the final DAL.
     for (auto ArgPtr : AllocatedArgs)
       Entry->AddSynthesizedArg(ArgPtr);
   }
 
   return *Entry;

clang/lib/Driver/ToolChain.cpp

llvm::opt::DerivedArgList *ToolChain::TranslateOpenMPTargetArgs(
 
   if (Modified)
     return DAL;
 
   delete DAL;
   return nullptr;
 }
 
-void ToolChain::TranslateXarchArgs(const llvm::opt::DerivedArgList &Args,
-                                   llvm::opt::Arg *&A,
-                                   llvm::opt::DerivedArgList *DAL) const {
+void ToolChain::TranslateXarchArgs(
+    const llvm::opt::DerivedArgList &Args, llvm::opt::Arg *&A,
+    llvm::opt::DerivedArgList *DAL,
+    SmallVectorImpl<llvm::opt::Arg *> *AllocatedArgs) const {
   const OptTable &Opts = getDriver().getOpts();
-  unsigned Index = Args.getBaseArgs().MakeIndex(A->getValue(1));
+  unsigned ValuePos = 1;
+  if (A->getOption().matches(options::OPT_Xarch_device) ||
+      A->getOption().matches(options::OPT_Xarch_host))
+    ValuePos = 0;
+
+  unsigned Index = Args.getBaseArgs().MakeIndex(A->getValue(ValuePos));
   unsigned Prev = Index;
   std::unique_ptr<llvm::opt::Arg> XarchArg(Opts.ParseOneArg(Args, Index));
 
   // If the argument parsing failed or more than one argument was
   // consumed, the -Xarch_ argument's parameter tried to consume
   // extra arguments. Emit an error and ignore.
   //
   // We also want to disallow any options which would alter the
   // driver behavior; that isn't going to work in our model. We
   // use isDriverOption() as an approximation, although things
   // like -O4 are going to slip through.
   if (!XarchArg || Index > Prev + 1) {
     getDriver().Diag(diag::err_drv_invalid_Xarch_argument_with_args)
         << A->getAsString(Args);
     return;
   } else if (XarchArg->getOption().hasFlag(options::DriverOption)) {
     getDriver().Diag(diag::err_drv_invalid_Xarch_argument_isdriver)
         << A->getAsString(Args);
     return;
   }
   XarchArg->setBaseArg(A);
   A = XarchArg.release();
+  if (!AllocatedArgs)
     DAL->AddSynthesizedArg(A);
+  else
+    AllocatedArgs->push_back(A);
+}
+
+llvm::opt::DerivedArgList *ToolChain::TranslateXarchArgs(
+    const llvm::opt::DerivedArgList &Args, StringRef BoundArch,
+    Action::OffloadKind OFK,
+    SmallVectorImpl<llvm::opt::Arg *> *AllocatedArgs) const {
+  DerivedArgList *DAL = new DerivedArgList(Args.getBaseArgs());
+  bool Modified = false;
+
+  bool IsGPU = OFK == Action::OFK_Cuda || OFK == Action::OFK_HIP;
+  for (Arg *A : Args) {
+    bool NeedTrans = false;
+    bool Skip = false;
+    if (A->getOption().matches(options::OPT_Xarch_device)) {
+      NeedTrans = IsGPU;
+      Skip = !IsGPU;
+    } else if (A->getOption().matches(options::OPT_Xarch_host)) {
+      NeedTrans = !IsGPU;
+      Skip = IsGPU;
+    } else if (A->getOption().matches(options::OPT_Xarch__) && IsGPU) {
+      // Do not translate -Xarch_ options for non CUDA/HIP toolchain since
+      // they may need special translation.
+      // Skip this argument unless the architecture matches BoundArch
+      if (BoundArch.empty() || A->getValue(0) != BoundArch)
+        Skip = true;
+      else
+        NeedTrans = true;
+    }
+    if (NeedTrans || Skip)
+      Modified = true;
+    if (NeedTrans)
+      TranslateXarchArgs(Args, A, DAL, AllocatedArgs);
+    if (!Skip)
+      DAL->append(A);
+  }
+
+  if (Modified)
+    return DAL;
+
+  delete DAL;
+  return nullptr;
 }

clang/lib/Driver/ToolChains/Cuda.cpp

if (DeviceOffloadKind == Action::OFK_OpenMP) {
     if (Arch.empty())
       DAL->AddJoinedArg(nullptr, Opts.getOption(options::OPT_march_EQ),
                         CLANG_OPENMP_NVPTX_DEFAULT_ARCH);
 
     return DAL;
   }
 
   for (Arg *A : Args) {
-    if (A->getOption().matches(options::OPT_Xarch__)) {
-      // Skip this argument unless the architecture matches BoundArch
-      if (BoundArch.empty() || A->getValue(0) != BoundArch)
-        continue;
-      TranslateXarchArgs(Args, A, DAL);
-    }
     DAL->append(A);
   }
 
   if (!BoundArch.empty()) {
     DAL->eraseArg(options::OPT_march_EQ);
     DAL->AddJoinedArg(nullptr, Opts.getOption(options::OPT_march_EQ), BoundArch);
   }
   return DAL;

clang/lib/Driver/ToolChains/HIP.cpp

HIPToolChain::TranslateArgs(const llvm::opt::DerivedArgList &Args,
   DerivedArgList *DAL =
       HostTC.TranslateArgs(Args, BoundArch, DeviceOffloadKind);
   if (!DAL)
     DAL = new DerivedArgList(Args.getBaseArgs());
 
   const OptTable &Opts = getDriver().getOpts();
 
   for (Arg *A : Args) {
-    if (A->getOption().matches(options::OPT_Xarch__)) {
-      // Skip this argument unless the architecture matches BoundArch.
-      if (BoundArch.empty() || A->getValue(0) != BoundArch)
-        continue;
-      TranslateXarchArgs(Args, A, DAL);
-    }
     DAL->append(A);
   }
 
   if (!BoundArch.empty()) {
     DAL->eraseArg(options::OPT_march_EQ);
     DAL->AddJoinedArg(nullptr, Opts.getOption(options::OPT_march_EQ), BoundArch);
   }
 

clang/test/Driver/hip-options.hip

 // REQUIRES: clang-driver
 // REQUIRES: x86-registered-target
 // REQUIRES: amdgpu-registered-target
 
 // RUN: %clang -### -x hip --gpu-max-threads-per-block=1024 %s 2>&1 | FileCheck %s
 
 // Check that there are commands for both host- and device-side compilations.
 //
 // CHECK: clang{{.*}}" "-cc1" {{.*}} "-fcuda-is-device"
 // CHECK-SAME: "--gpu-max-threads-per-block=1024"
 
 // RUN: %clang -### -x hip -target x86_64-pc-windows-msvc -fms-extensions \
 // RUN:   -mllvm -amdgpu-early-inline-all=true  %s 2>&1 | \
 // RUN:   FileCheck -check-prefix=MLLVM %s
 // MLLVM-NOT: "-mllvm"{{.*}}"-amdgpu-early-inline-all=true"{{.*}}"-mllvm"{{.*}}"-amdgpu-early-inline-all=true"
+
+// RUN: %clang -### -Xarch_device -g -nogpulib --cuda-gpu-arch=gfx900 \
+// RUN:   -Xarch_device -fcf-protection=branch \
+// RUN:   --cuda-gpu-arch=gfx906  %s 2>&1 | FileCheck -check-prefix=DEV %s
+// DEV: clang{{.*}} "-fcuda-is-device" {{.*}} "-debug-info-kind={{.*}}" {{.*}} "-fcf-protection=branch"
+// DEV: clang{{.*}} "-fcuda-is-device" {{.*}} "-debug-info-kind={{.*}}" {{.*}} "-fcf-protection=branch"
+// DEV-NOT: clang{{.*}} {{.*}} "-debug-info-kind={{.*}}"
+
+// RUN: %clang -### -Xarch_host -g -nogpulib --cuda-gpu-arch=gfx900 \
+// RUN:   --cuda-gpu-arch=gfx906  %s 2>&1 | FileCheck -check-prefix=HOST %s
+// HOST-NOT: clang{{.*}} "-fcuda-is-device" {{.*}} "-debug-info-kind={{.*}}"
+// HOST-NOT: clang{{.*}} "-fcuda-is-device" {{.*}} "-debug-info-kind={{.*}}"
+// HOST: clang{{.*}} "-debug-info-kind={{.*}}"