[OpenMP] Add options to only compile the host or device when offloading

[jhuber6]

Summary

OpenMP recently moved to the new offloading driver, this had the effect
of making it more difficult to inspect intermediate code for the device.
This patch adds -foffload-host-only and -foffload-device-only to
control which sides get compiled. This will allow users to more easily
inspect output without needing the temp files.

[jhuber6]

Moving some stuff.

[jhuber6]

Small change, returning an empty action type should let us stop without an extra check.

[tra]

clang/include/clang/Driver/Options.td

2527–2530

We should probably alias --cuda-host-only/--cuda-device-only to these.

[jhuber6]

ping

clang/include/clang/Driver/Options.td

2527–2530

Unfortunately I can't make these a strict alias for -foffload-host-only/-foffload-device-only without breaking their current usage. I could just check them along with these new versions, but that makes it a little cluttered.

[tra]

clang/include/clang/Driver/Options.td

2527–2530

Could you give me an example why we can use just one set of options to control host-only and device-only compilation?

Is that because we also have --cuda-compile-host-device?
Or is it because -f options have -fno- variants to override them, while --cuda-host/device-only don't, so we can't just mark them as an alias in Options.td?
To think of it, those are just facets of the same issue -- difference in the options syntax.
I do not see semantic differences between the existing options and -foffload-*-only.

E.g. I believe you could've used --cuda-host-only and --cuda-device-only instead of -foffload-*-only without the loss of functionality of your patch.

[jhuber6]

clang/include/clang/Driver/Options.td

2527–2530

I could have used those options, but this applies in general to the new driver, so it wouldn't be intuitive if we used --cuda-device-only when offloading to x86_64 with OpenMP for example. The problem is that the --cuda-device-only is already used in the current offloading driver so if I just make it an alias to this option it won't work. I was hoping to avoid touching the old driver, but I guess I could go back and replace all uses of --cuda-host/device-only with these new options. Also I did forget to add the fno- variants for these and use them correctly. I'm not sure if there's an explicit reason to, but everything else in the OpenMP world uses -f prefixes so I'm mostly just sticking with that.

[jhuber6]

Adding -fno variants and replacing the uses of --cuda-host-only with and alias and this flag.

[tra]

clang/include/clang/Driver/Options.td

2527–2530

it wouldn't be intuitive if we used --cuda-device-only when offloading to x86_64 with OpenMP

On the other hand, adding another set of options replicating the functionality creates more opportunities for conflicting usage. E.g. what should we do if user specifies --cuda-device-only and -foffload-device-only ? We then need to diagnose that in a sensible way, at the very least.

In general, we have been transitioning some options that used to be CUDA-specific into more general variants. Usually --cuda* -> --offload* (--cuda-gpu-arch->--offload-arch) or --gpu*. (-fcuda-rdc -> -fgpu-rdc)
Picking the offloading side should follow the same pattern.

Speaking of names and renaming. Perhaps -f is not the best choice here. -f options are conventionally used to control code generation parameters. -foffload-*-only mostly controls the driver behavior. Perhaps it would make sense to call them --offload-host-only, --offload-device-only and --offload-host-device and alias them to the matching --cuda* counterparts (or vice-versa + search/replace their use in the code).

[jhuber6]

Updating to not use -f/-fno versions.

[tra]

clang/include/clang/Driver/Options.td

2527

We should be using "--" for the new options.

clang/lib/Driver/Driver.cpp

4205

Comment may need updating.

4215–4216

This will not always be correct. E.g. --offload-host-only --offload-host-device would be true here, but we would still want to compile for both and device.

Is there a reason we can no longer rely on HostAction?

4224

This could use a comment. I don't quite understand what we're doing here and why.
Only doing host-side preprocessing if -E is passed?

4279

Same problem as with the host-checking above.

[jhuber6]

clang/include/clang/Driver/Options.td

2527

What's the reason for preferring -- over -? Is it just because -o can bind to output?

clang/lib/Driver/Driver.cpp

4215–4216

Guess I should just use the last argument. Host action is used below, can probably merge these if statements.

4224

OpenMP requires host IR for the device compile, but -E doesn't generate one, but we don't use it for preprocessing.

[jhuber6]

Making suggested changes.

[tra]

clang/include/clang/Driver/Options.td

2527

Convention, I guess. Legacy (e.g. -nostdlib ) and single-letter options (e.g. -o -m, -f ) use single dash. Long options typically use double-dash.

clang/lib/Driver/Driver.cpp

4215–4216

I guess the general idea is to avoid the ambiguity about what controls the behavior of the function. Is that the Args, or the HostAction?
Ideally I'd prefer to parse command line options once, save results somewhere we could use them and then use those flargs to control the behavior, regardless of which options were used to specify it. E.g. CUDA/HIPActionBuilder classes have these member fields:

  bool CompileHostOnly = false;
  bool CompileDeviceOnly = false;

...
      Arg *PartialCompilationArg = Args.getLastArg(
          options::OPT_cuda_host_only, options::OPT_cuda_device_only,
          options::OPT_cuda_compile_host_device);
      CompileHostOnly = PartialCompilationArg &&
                        PartialCompilationArg->getOption().matches(
                            options::OPT_cuda_host_only);
      CompileDeviceOnly = PartialCompilationArg &&
                          PartialCompilationArg->getOption().matches(
                              options::OPT_cuda_device_only);
...

[jhuber6]

clang/lib/Driver/Driver.cpp

4215–4216

I more or less copied that below, but I suppose we are recalculating them each phase. We don't have a monolithic class with my new implementation, but I suppose I could add these to the Compilation or something if we don't want to recalculate it. Beside that, does anything else seem amiss?

[tra]

LGTM with a minor nit.

clang/lib/Driver/Driver.cpp

4226

Nit: (!isa<CompileJobAction>(HostAction) && PL.back() != phases::Preprocess) -> !(isa<CompileJobAction>(HostAction) || PL.back() == phases::Preprocess)

It's a bit easier to understand that way, IMO. We could also return early if HostOnly is set and make this condition simpler.

[jhuber6]

Rebase after landing the cuda support for the new driver.

clang/include/clang/Driver/Options.td

 def c : Flag<["-"], "c">, Flags<[NoXarchOption, FlangOption]>, Group<Action_Group>,
   HelpText<"Only run preprocess, compile, and assemble steps">;
 def fconvergent_functions : Flag<["-"], "fconvergent-functions">, Group<f_Group>, Flags<[CC1Option]>,
   HelpText<"Assume functions may be convergent">;
 
 def gpu_use_aux_triple_only : Flag<["--"], "gpu-use-aux-triple-only">,
   InternalDriverOpt, HelpText<"Prepare '-aux-triple' only without populating "
                               "'-aux-target-cpu' and '-aux-target-feature'.">;
-def cuda_device_only : Flag<["--"], "cuda-device-only">,
-  HelpText<"Compile CUDA code for device only">;
-def cuda_host_only : Flag<["--"], "cuda-host-only">,
-  HelpText<"Compile CUDA code for host only.  Has no effect on non-CUDA "
-           "compilations.">;
-def cuda_compile_host_device : Flag<["--"], "cuda-compile-host-device">,
-  HelpText<"Compile CUDA code for both host and device (default).  Has no "
-           "effect on non-CUDA compilations.">;
 def cuda_include_ptx_EQ : Joined<["--"], "cuda-include-ptx=">, Flags<[NoXarchOption]>,
   HelpText<"Include PTX for the following GPU architecture (e.g. sm_35) or 'all'. May be specified more than once.">;
 def no_cuda_include_ptx_EQ : Joined<["--"], "no-cuda-include-ptx=">, Flags<[NoXarchOption]>,
   HelpText<"Do not include PTX for the following GPU architecture (e.g. sm_35) or 'all'. May be specified more than once.">;
 def offload_arch_EQ : Joined<["--"], "offload-arch=">, Flags<[NoXarchOption]>,
   HelpText<"CUDA offloading device architecture (e.g. sm_35), or HIP offloading target ID in the form of a "
            "device architecture followed by target ID features delimited by a colon. Each target ID feature "
            "is a pre-defined string followed by a plus or minus sign (e.g. gfx908:xnack+:sramecc-).  May be "
def fopenmp_target_new_runtime : Flag<["-"], "fopenmp-target-new-runtime">,
   Group<f_Group>, Flags<[CC1Option, HelpHidden]>;
 def fno_openmp_target_new_runtime : Flag<["-"], "fno-openmp-target-new-runtime">,
   Group<f_Group>, Flags<[CC1Option, HelpHidden]>;
 defm openmp_optimistic_collapse : BoolFOption<"openmp-optimistic-collapse",
   LangOpts<"OpenMPOptimisticCollapse">, DefaultFalse,
   PosFlag<SetTrue, [CC1Option]>, NegFlag<SetFalse>, BothFlags<[NoArgumentUnused, HelpHidden]>>;
 def static_openmp: Flag<["-"], "static-openmp">,
   HelpText<"Use the static host OpenMP runtime while linking.">;
+def foffload_device_only : Flag<["-"], "foffload-device-only">,
+  HelpText<"Only compile for the offloading device.">;
+def fno_offload_device_only : Flag<["-"], "fno-offload-device-only">,
+  HelpText<"Only compile for the offloading device.">;
+def foffload_host_only : Flag<["-"], "foffload-host-only">,
+  HelpText<"Only compile for the offloading host.">;
+def fno_offload_host_only : Flag<["-"], "fno-offload-host-only">,
+  HelpText<"Only compile for the offloading host.">;
+def cuda_device_only : Flag<["--"], "cuda-device-only">, Alias<foffload_device_only>,
+  HelpText<"Compile CUDA code for device only">;
+def cuda_host_only : Flag<["--"], "cuda-host-only">, Alias<foffload_host_only>,
+  HelpText<"Compile CUDA code for host only.  Has no effect on non-CUDA "
+           "compilations.">;
+def cuda_compile_host_device : Flag<["--"], "cuda-compile-host-device">,
+  HelpText<"Compile CUDA code for both host and device (default).  Has no "
+           "effect on non-CUDA compilations.">;
 def fopenmp_new_driver : Flag<["-"], "fopenmp-new-driver">, Flags<[CC1Option]>, Group<Action_Group>,
   HelpText<"Use the new driver for OpenMP offloading.">;
 def fno_openmp_new_driver : Flag<["-"], "fno-openmp-new-driver">, Flags<[CC1Option]>, Group<Action_Group>,
   HelpText<"Don't use the new driver for OpenMP offloading.">;
 def fno_optimize_sibling_calls : Flag<["-"], "fno-optimize-sibling-calls">, Group<f_Group>, Flags<[CC1Option]>,
   HelpText<"Disable tail call optimization, keeping the call stack accurate">,
   MarshallingInfoFlag<CodeGenOpts<"DisableTailCalls">>;
 def foptimize_sibling_calls : Flag<["-"], "foptimize-sibling-calls">, Group<f_Group>;

clang/lib/Driver/Driver.cpp

bool initialize() override {
       }
 
       ToolChains.push_back(
           AssociatedOffloadKind == Action::OFK_Cuda
               ? C.getSingleOffloadToolChain<Action::OFK_Cuda>()
               : C.getSingleOffloadToolChain<Action::OFK_HIP>());
 
       Arg *PartialCompilationArg = Args.getLastArg(
-          options::OPT_cuda_host_only, options::OPT_cuda_device_only,
+          options::OPT_foffload_host_only, options::OPT_foffload_device_only,
           options::OPT_cuda_compile_host_device);
-      CompileHostOnly = PartialCompilationArg &&
-                        PartialCompilationArg->getOption().matches(
-                            options::OPT_cuda_host_only);
-      CompileDeviceOnly = PartialCompilationArg &&
-                          PartialCompilationArg->getOption().matches(
-                              options::OPT_cuda_device_only);
+      CompileHostOnly =
+          PartialCompilationArg && PartialCompilationArg->getOption().matches(
+                                       options::OPT_foffload_host_only);
+      CompileDeviceOnly =
+          PartialCompilationArg && PartialCompilationArg->getOption().matches(
+                                       options::OPT_foffload_device_only);
       EmitLLVM = Args.getLastArg(options::OPT_emit_llvm);
       EmitAsm = Args.getLastArg(options::OPT_S);
       FixedCUID = Args.getLastArgValue(options::OPT_cuid_EQ);
       if (Arg *A = Args.getLastArg(options::OPT_fuse_cuid_EQ)) {
         StringRef UseCUIDStr = A->getValue();
         UseCUID = llvm::StringSwitch<UseCUIDKind>(UseCUIDStr)
                       .Case("hash", CUID_Hash)
                       .Case("random", CUID_Random)
for (phases::ID Phase : PL) {
       }
 
       if (Phase == phases::Precompile && ExtractAPIAction) {
         ExtractAPIAction->addHeaderInput(Current);
         Current = nullptr;
         break;
       }
 
-      // Try to build the offloading actions and add the result as a dependency
-      // to the host.
-      if (UseNewOffloadingDriver)
-        Current = BuildOffloadingActions(C, Args, I, Current);
-
       // FIXME: Should we include any prior module file outputs as inputs of
       // later actions in the same command line?
 
       // Otherwise construct the appropriate action.
       Action *NewCurrent = ConstructPhaseAction(C, Args, Phase, Current);
 
       // We didn't create a new action, so we will just move to the next phase.
       if (NewCurrent == Current)
         continue;
 
       if (auto *HMA = dyn_cast<HeaderModulePrecompileJobAction>(NewCurrent))
         HeaderModuleAction = HMA;
       else if (auto *EAA = dyn_cast<ExtractAPIJobAction>(NewCurrent))
         ExtractAPIAction = EAA;
 
       Current = NewCurrent;
 
       // Use the current host action in any of the offloading actions, if
       // required.
       if (!UseNewOffloadingDriver)
         if (OffloadBuilder.addHostDependenceToDeviceActions(Current, InputArg))
           break;
 
+      // Try to build the offloading actions and add the result as a dependency
+      // to the host.
+      if (UseNewOffloadingDriver)
+        Current = BuildOffloadingActions(C, Args, I, Current);
+
       if (Current->getType() == types::TY_Nothing)
         break;
     }
 
     // If we ended with something, add to the output list.
     if (Current)
       Actions.push_back(Current);
 
Actions.push_back(
         C.MakeAction<PrecompileJobAction>(InputAc, types::TY_Nothing));
     for (auto &I : Inputs)
       I.second->claim();
   }
 
   // Claim ignored clang-cl options.
   Args.ClaimAllArgs(options::OPT_cl_ignored_Group);
 
   // Claim --cuda-host-only and --cuda-compile-host-device, which may be passed
   // to non-CUDA compilations and should not trigger warnings there.
-  Args.ClaimAllArgs(options::OPT_cuda_host_only);
+  Args.ClaimAllArgs(options::OPT_foffload_host_only);
   Args.ClaimAllArgs(options::OPT_cuda_compile_host_device);
 }
 
 Action *Driver::BuildOffloadingActions(Compilation &C,
                                        llvm::opt::DerivedArgList &Args,
                                        const InputTy &Input,
                                        Action *HostAction) const {
-  if (!isa<CompileJobAction>(HostAction))
+  if (Args.hasFlag(options::OPT_foffload_host_only,
+                   options::OPT_fno_offload_host_only, false))
     return HostAction;
 
-  OffloadAction::DeviceDependences DDeps;
-
   types::ID InputType = Input.first;
   const Arg *InputArg = Input.second;
 
+  auto PL = types::getCompilationPhases(*this, Args, InputType);
+
+  if (!isa<CompileJobAction>(HostAction) && PL.back() != phases::Preprocess)
+    return HostAction;
+
+  OffloadAction::DeviceDependences DDeps;
+
   const Action::OffloadKind OffloadKinds[] = {Action::OFK_OpenMP};
 
   for (Action::OffloadKind Kind : OffloadKinds) {
     SmallVector<const ToolChain *, 2> ToolChains;
     ActionList DeviceActions;
 
     auto TCRange = C.getOffloadToolChains(Kind);
     for (auto TI = TCRange.first, TE = TCRange.second; TI != TE; ++TI)
       ToolChains.push_back(TI->second);
 
     if (ToolChains.empty())
       continue;
 
     for (unsigned I = 0; I < ToolChains.size(); ++I)
       DeviceActions.push_back(C.MakeAction<InputAction>(*InputArg, InputType));
 
     if (DeviceActions.empty())
       return HostAction;
 
-    auto PL = types::getCompilationPhases(*this, Args, InputType);
-
     for (phases::ID Phase : PL) {
       if (Phase == phases::Link) {
         assert(Phase == PL.back() && "linking must be final compilation step.");
         break;
       }
 
       auto TC = ToolChains.begin();
       for (Action *&A : DeviceActions) {
         A = ConstructPhaseAction(C, Args, Phase, A, Kind);
 
-        if (isa<CompileJobAction>(A) && Kind == Action::OFK_OpenMP) {
+        if (isa<CompileJobAction>(A) && isa<CompileJobAction>(HostAction) &&
+            Kind == Action::OFK_OpenMP) {
           HostAction->setCannotBeCollapsedWithNextDependentAction();
           OffloadAction::HostDependence HDep(
               *HostAction, *C.getSingleOffloadToolChain<Action::OFK_Host>(),
               /*BourdArch=*/nullptr, Action::OFK_OpenMP);
           OffloadAction::DeviceDependences DDep;
           DDep.add(*A, **TC, /*BoundArch=*/nullptr, Kind);
           A = C.MakeAction<OffloadAction>(HDep, DDep);
         }
         ++TC;
       }
     }
 
     auto TC = ToolChains.begin();
     for (Action *A : DeviceActions) {
       DDeps.add(*A, **TC, /*BoundArch=*/nullptr, Kind);
       TC++;
     }
   }
 
+  if (Args.hasFlag(options::OPT_foffload_device_only,
+                   options::OPT_fno_offload_device_only, false))
+    return C.MakeAction<OffloadAction>(DDeps, types::TY_Nothing);
+
   OffloadAction::HostDependence HDep(
       *HostAction, *C.getSingleOffloadToolChain<Action::OFK_Host>(),
       /*BoundArch=*/nullptr, DDeps);
   return C.MakeAction<OffloadAction>(HDep, DDeps);
 }
 
 Action *Driver::ConstructPhaseAction(
     Compilation &C, const ArgList &Args, phases::ID Phase, Action *Input,

clang/test/Driver/openmp-offload-gpu-new.c

 ///
 /// Perform several driver tests for OpenMP offloading
 ///
 
 // REQUIRES: x86-registered-target
-// REQUIRES: powerpc-registered-target
 // REQUIRES: nvptx-registered-target
 // REQUIRES: amdgpu-registered-target
 
 // RUN:   %clang -### --target=x86_64-unknown-linux-gnu -fopenmp -fopenmp-targets=nvptx64-nvidia-cuda \
 // RUN:          -Xopenmp-target=nvptx64-nvidia-cuda -march=sm_52 \
 // RUN:          --libomptarget-nvptx-bc-path=%S/Inputs/libomptarget/libomptarget-nvptx-test.bc %s 2>&1 \
 // RUN:   | FileCheck %s
 

 // CHECK-EMIT-LLVM-IR: clang{{.*}}"-cc1"{{.*}}"-triple" "nvptx64-nvidia-cuda"{{.*}}"-emit-llvm"
 
 // RUN:   %clang -### -fopenmp=libomp -fopenmp-targets=nvptx64-nvidia-cuda -Xopenmp-target=nvptx64-nvida-cuda -march=sm_70 \
 // RUN:          --libomptarget-nvptx-bc-path=%S/Inputs/libomptarget/libomptarget-new-nvptx-test.bc \
 // RUN:          -no-canonical-prefixes -nogpulib %s -o openmp-offload-gpu 2>&1 \
 // RUN:   | FileCheck -check-prefix=DRIVER_EMBEDDING %s
 
 // DRIVER_EMBEDDING: -fembed-offload-object=[[CUBIN:.*\.cubin]],openmp,nvptx64-nvidia-cuda,sm_70
+
+// RUN:   %clang -### --target=x86_64-unknown-linux-gnu -ccc-print-bindings -fopenmp -fopenmp-targets=nvptx64-nvidia-cuda \
+// RUN:     -foffload-host-only -nogpulib %s 2>&1 | FileCheck %s --check-prefix=CHECK-HOST-ONLY
+// CHECK-HOST-ONLY: "x86_64-unknown-linux-gnu" - "clang", inputs: ["[[INPUT:.*]]"], output: "[[OUTPUT:.*]]"
+// CHECK-HOST-ONLY: "x86_64-unknown-linux-gnu" - "Offload::Linker", inputs: ["[[OUTPUT]]"], output: "a.out"
+
+// RUN:   %clang -### --target=x86_64-unknown-linux-gnu -ccc-print-bindings -fopenmp -fopenmp-targets=nvptx64-nvidia-cuda \
+// RUN:     -foffload-device-only -nogpulib %s 2>&1 | FileCheck %s --check-prefix=CHECK-DEVICE-ONLY
+// CHECK-DEVICE-ONLY: "x86_64-unknown-linux-gnu" - "clang", inputs: ["[[INPUT:.*]]"], output: "[[HOST_BC:.*]]"
+// CHECK-DEVICE-ONLY: "nvptx64-nvidia-cuda" - "clang", inputs: ["[[INPUT]]", "[[HOST_BC]]"], output: "[[DEVICE_ASM:.*]]"
+// CHECK-DEVICE-ONLY: "nvptx64-nvidia-cuda" - "NVPTX::Assembler", inputs: ["[[DEVICE_ASM]]"], output: "{{.*}}-openmp-nvptx64-nvidia-cuda.o"
+
+// RUN:   %clang -### --target=x86_64-unknown-linux-gnu -ccc-print-bindings -fopenmp -fopenmp-targets=nvptx64-nvidia-cuda \
+// RUN:     -foffload-device-only -E -nogpulib %s 2>&1 | FileCheck %s --check-prefix=CHECK-DEVICE-ONLY-PP
+// CHECK-DEVICE-ONLY-PP: "nvptx64-nvidia-cuda" - "clang", inputs: ["[[INPUT:.*]]"], output: "-"