[Clang] Make enabling the new driver more generic

[jhuber6]

Summary

In preparation for allowing other offloading kinds to use the new driver
a new opt-in flag -foffload-new-driver is added. This is distinct from
the existing -fopenmp-new-driver because OpenMP will soon use the new
driver by default while the others should not.

[jhuber6]

Fix misplaced logic symbol that broke tests.

[jhuber6]

Rebase.

[jhuber6]

ping

[yaxunl]

clang/lib/Driver/Driver.cpp

3979–3982

If OPT_fopenmp_new_driver is on by default, then OPT_foffload_new_driver will have no effect. Is this intended? Should OPT_fopenmp_new_driver be conditioned for OpenMP offloading only?

[jhuber6]

clang/lib/Driver/Driver.cpp

3979–3982

Yeah, I need to rework the logic here once OpenMP moves to the new driver by default. I can probably require something like.

(C.isOffloadingHostKind(Action::OFK_OpenMP) && !Args.hasArg(options::OPT_fno_offload_new_driver) || Args.hasArg(options::OPT_fopenmp_new_driver);

Currently working on the patch to make this default for OpenMP so I'll be able to update this after that.

[jhuber6]

Rebase & update after making the new driver default for OpenMP.

[jhuber6]

Fix

[jhuber6]

Removing unnecessary change

[yaxunl]

should we have a test to show the effect of -foffload-new-driver on -ccc-print-phases for CUDA/HIP?

[jhuber6]

In D123325#3459108, @yaxunl wrote:

should we have a test to show the effect of -foffload-new-driver on -ccc-print-phases for CUDA/HIP?

We probably should, it's tested w/ OpenMP but we should check the bound architectures and such. I can add one.

[jhuber6]

In D123325#3459108, @yaxunl wrote:

should we have a test to show the effect of -foffload-new-driver on -ccc-print-phases for CUDA/HIP?

I decided to add the test in D120272 because this file only adds support for the option.

[jhuber6]

Ping and update.

[yaxunl]

LGTM. Thanks.

[tra]

clang/include/clang/Driver/Options.td

2535–2538

Nit: Can we combine that into a BoolFOption instead of using Flag ?

Also, considering that it's an option which controls the driver's behavior, not just tweaks code generation, perhaps it should be just --[no]offload-new-driver.

[jhuber6]

clang/include/clang/Driver/Options.td

2535–2538

The -fopenmp variant was in LLVM-14 so that should stay at least. I could change it to be --offload (Although this does tweak code generation in a later patch to make the offloading entries). I most just used -f because all the other OpenMP options use -f. I could definitely use BoolFOption but I don't think it saves too much in this situation.

[tra]

clang/include/clang/Driver/Options.td

2535–2538

Naming is hard. :-) Real world things don't want to fit into neat categories.

I think --offload* would fit the general pattern of transitioning niche features (be it CUDA,HIP, OpenMP) into something more general (offload, GPU).

[jhuber6]

clang/include/clang/Driver/Options.td

2535–2538

Sure, I'll change it to just be offload.

[jhuber6]

Changin -f[no-]offload-new-driver into --[no-]offload-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_new_driver : Flag<["-"], "foffload-new-driver">, Flags<[CC1Option]>, Group<Action_Group>,
+  HelpText<"Use the new driver for offloading compilation.">;
+def fno_offload_new_driver : Flag<["-"], "fno-offload-new-driver">, Flags<[CC1Option]>, Group<Action_Group>,
+  HelpText<"Don't Use the new driver for offloading compilation.">;
 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.">;
+  Alias<fno_offload_new_driver>, 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>;
 defm escaping_block_tail_calls : BoolFOption<"escaping-block-tail-calls",
   CodeGenOpts<"NoEscapingBlockTailCalls">, DefaultFalse,
   NegFlag<SetTrue, [CC1Option]>, PosFlag<SetFalse>>;
 def force__cpusubtype__ALL : Flag<["-"], "force_cpusubtype_ALL">;

clang/lib/Driver/Driver.cpp

if (Arg *A = Args.getLastArg(options::OPT__SLASH_o)) {
     }
   }
 
   handleArguments(C, Args, Inputs, Actions);
 
   // Builder to be used to build offloading actions.
   OffloadingActionBuilder OffloadBuilder(C, Args, Inputs);
 
+  bool UseNewOffloadingDriver =
+      (C.isOffloadingHostKind(Action::OFK_OpenMP) &&
+      Args.hasFlag(options::OPT_fopenmp_new_driver,
+                   options::OPT_fno_offload_new_driver, true)) ||
+      Args.hasFlag(options::OPT_foffload_new_driver,
+          options::OPT_fno_offload_new_driver, false);
+
   // Construct the actions to perform.
   HeaderModulePrecompileJobAction *HeaderModuleAction = nullptr;
   ExtractAPIJobAction *ExtractAPIAction = nullptr;
   ActionList LinkerInputs;
   ActionList MergerInputs;
 
-  bool UseNewOffloadingDriver =
-      C.isOffloadingHostKind(Action::OFK_OpenMP) &&
-      Args.hasFlag(options::OPT_fopenmp_new_driver,
-                   options::OPT_fno_openmp_new_driver, true);
-
   for (auto &I : Inputs) {
     types::ID InputType = I.first;
     const Arg *InputArg = I.second;
 
     auto PL = types::getCompilationPhases(*this, Args, InputType);
     if (PL.empty())
       continue;
 
void Driver::BuildActions(Compilation &C, DerivedArgList &Args,
   if (!LinkerInputs.empty()) {
     if (!UseNewOffloadingDriver)
       if (Action *Wrapper = OffloadBuilder.makeHostLinkAction())
         LinkerInputs.push_back(Wrapper);
     Action *LA;
     // Check if this Linker Job should emit a static library.
     if (ShouldEmitStaticLibrary(Args)) {
       LA = C.MakeAction<StaticLibJobAction>(LinkerInputs, types::TY_Image);
-    } else if (UseNewOffloadingDriver &&
-               C.getActiveOffloadKinds() != Action::OFK_None) {
+    } else if (UseNewOffloadingDriver) {
       LA = C.MakeAction<LinkerWrapperJobAction>(LinkerInputs, types::TY_Image);
       LA->propagateHostOffloadInfo(C.getActiveOffloadKinds(),
                                    /*BoundArch=*/nullptr);
     } else {
       LA = C.MakeAction<LinkJobAction>(LinkerInputs, types::TY_Image);
     }
     if (!UseNewOffloadingDriver)
       LA = OffloadBuilder.processHostLinkAction(LA);

clang/lib/Driver/ToolChains/Clang.cpp

void Clang::ConstructJob(Compilation &C, const JobAction &JA,
   bool IsHIP = JA.isOffloading(Action::OFK_HIP);
   bool IsHIPDevice = JA.isDeviceOffloading(Action::OFK_HIP);
   bool IsOpenMPDevice = JA.isDeviceOffloading(Action::OFK_OpenMP);
   bool IsHeaderModulePrecompile = isa<HeaderModulePrecompileJobAction>(JA);
   bool IsExtractAPI = isa<ExtractAPIJobAction>(JA);
   bool IsDeviceOffloadAction = !(JA.isDeviceOffloading(Action::OFK_None) ||
                                  JA.isDeviceOffloading(Action::OFK_Host));
   bool IsHostOffloadingAction =
-      JA.isHostOffloading(Action::OFK_OpenMP) &&
-      !Args.hasArg(options::OPT_fno_openmp_new_driver);
+      (JA.isHostOffloading(Action::OFK_OpenMP) &&
+       Args.hasFlag(options::OPT_fopenmp_new_driver,
+         options::OPT_fno_offload_new_driver, true)) ||
+      Args.hasFlag(options::OPT_foffload_new_driver,
+          options::OPT_fno_offload_new_driver, false);
   bool IsUsingLTO = D.isUsingLTO(IsDeviceOffloadAction);
   auto LTOMode = D.getLTOMode(IsDeviceOffloadAction);
 
   // A header module compilation doesn't have a main input file, so invent a
   // fake one as a placeholder.
   const char *ModuleName = [&]{
     auto *ModuleNameArg = Args.getLastArg(options::OPT_fmodule_name_EQ);
     return ModuleNameArg ? ModuleNameArg->getValue() : "";
if (isa<AnalyzeJobAction>(JA)) {
     // the use-list order, since serialization to bitcode is part of the flow.
     if (JA.getType() == types::TY_LLVM_BC)
       CmdArgs.push_back("-emit-llvm-uselists");
 
     if (IsUsingLTO) {
       // Only AMDGPU supports device-side LTO.
       if (IsDeviceOffloadAction &&
           !Args.hasFlag(options::OPT_fopenmp_new_driver,
-                        options::OPT_fno_openmp_new_driver, true) &&
+                        options::OPT_fno_offload_new_driver, true) &&
+          !Args.hasFlag(options::OPT_foffload_new_driver,
+            options::OPT_fno_offload_new_driver, false) &&
           !Triple.isAMDGPU()) {
         D.Diag(diag::err_drv_unsupported_opt_for_target)
             << Args.getLastArg(options::OPT_foffload_lto,
                                options::OPT_foffload_lto_EQ)
                    ->getAsString(Args)
             << Triple.getTriple();
       } else {
         assert(LTOMode == LTOK_Full || LTOMode == LTOK_Thin);