[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

2535–2538

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

[jhuber6]

ping

clang/include/clang/Driver/Options.td

2535–2538

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

2535–2538

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

2535–2538

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

2535–2538

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

2535

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

clang/lib/Driver/Driver.cpp

4205

Comment may need updating.

4215

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?

4223

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?

4278

Same problem as with the host-checking above.

[jhuber6]

clang/include/clang/Driver/Options.td

2535

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

clang/lib/Driver/Driver.cpp

4215

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

4223

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

2535

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

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

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

4225

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 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 foffload_host_only : Flag<["-"], "foffload-host-only">,
+  HelpText<"Only compile for the offloading host.">;
 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

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_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.hasArg(options::OPT_foffload_host_only))
     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.hasArg(options::OPT_foffload_device_only))
+    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: "-"