[PATCH] D123325: [Clang] Make enabling the new driver more generic

[Joseph Huber via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG4e2b5a6693e2: [Clang] Make enabling the new driver more 
generic (authored by jhuber6).

Changed prior to commit:
  https://reviews.llvm.org/D123325?vs=425879=426035#toc

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D123325/new/

https://reviews.llvm.org/D123325

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/ToolChains/Clang.cpp


Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -4389,8 +4389,11 @@
   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_no_offload_new_driver, true)) ||
+  Args.hasFlag(options::OPT_offload_new_driver,
+   options::OPT_no_offload_new_driver, false);
   bool IsUsingLTO = D.isUsingLTO(IsDeviceOffloadAction);
   auto LTOMode = D.getLTOMode(IsDeviceOffloadAction);
 
@@ -4686,7 +4689,9 @@
   // Only AMDGPU supports device-side LTO.
   if (IsDeviceOffloadAction &&
   !Args.hasFlag(options::OPT_fopenmp_new_driver,
-options::OPT_fno_openmp_new_driver, true) &&
+options::OPT_no_offload_new_driver, true) &&
+  !Args.hasFlag(options::OPT_offload_new_driver,
+options::OPT_no_offload_new_driver, false) &&
   !Triple.isAMDGPU()) {
 D.Diag(diag::err_drv_unsupported_opt_for_target)
 << Args.getLastArg(options::OPT_foffload_lto,
Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -3976,17 +3976,19 @@
   // 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_no_offload_new_driver, true)) ||
+  Args.hasFlag(options::OPT_offload_new_driver,
+   options::OPT_no_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  : Inputs) {
 types::ID InputType = I.first;
 const Arg *InputArg = I.second;
@@ -4114,8 +4116,7 @@
 // Check if this Linker Job should emit a static library.
 if (ShouldEmitStaticLibrary(Args)) {
   LA = C.MakeAction(LinkerInputs, types::TY_Image);
-} else if (UseNewOffloadingDriver &&
-   C.getActiveOffloadKinds() != Action::OFK_None) {
+} else if (UseNewOffloadingDriver) {
   LA = C.MakeAction(LinkerInputs, types::TY_Image);
   LA->propagateHostOffloadInfo(C.getActiveOffloadKinds(),
/*BoundArch=*/nullptr);
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -2534,10 +2534,14 @@
   PosFlag, NegFlag, 
BothFlags<[NoArgumentUnused, HelpHidden]>>;
 def static_openmp: Flag<["-"], "static-openmp">,
   HelpText<"Use the static host OpenMP runtime while linking.">;
+def offload_new_driver : Flag<["--"], "offload-new-driver">, 
Flags<[CC1Option]>, Group,
+  HelpText<"Use the new driver for offloading compilation.">;
+def no_offload_new_driver : Flag<["--"], "no-offload-new-driver">, 
Flags<[CC1Option]>, Group,
+  HelpText<"Don't Use the new driver for offloading compilation.">;
 def fopenmp_new_driver : Flag<["-"], "fopenmp-new-driver">, 
Flags<[CC1Option]>, Group,
   HelpText<"Use the new driver for OpenMP offloading.">;
 def fno_openmp_new_driver : Flag<["-"], "fno-openmp-new-driver">, 
Flags<[CC1Option]>, Group,
-  HelpText<"Don't use the new driver for OpenMP offloading.">;
+  Alias, HelpText<"Don't use the new driver for OpenMP 
offloading.">;
 def fno_optimize_sibling_calls : Flag<["-"], "fno-optimize-sibling-calls">, 
Group, Flags<[CC1Option]>,
   HelpText<"Disable tail call optimization, 

[PATCH] D123325: [Clang] Make enabling the new driver more generic

[Joseph Huber via Phabricator via cfe-commits]

jhuber6 updated this revision to Diff 425879.
jhuber6 added a comment.

Changin `-f[no-]offload-new-driver` into `--[no-]offload-new-driver`


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D123325/new/

https://reviews.llvm.org/D123325

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/ToolChains/Clang.cpp


Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -4381,8 +4381,11 @@
   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_no_offload_new_driver, true)) ||
+  Args.hasFlag(options::OPT_offload_new_driver,
+  options::OPT_no_offload_new_driver, false);
   bool IsUsingLTO = D.isUsingLTO(IsDeviceOffloadAction);
   auto LTOMode = D.getLTOMode(IsDeviceOffloadAction);
 
@@ -4678,7 +4681,9 @@
   // Only AMDGPU supports device-side LTO.
   if (IsDeviceOffloadAction &&
   !Args.hasFlag(options::OPT_fopenmp_new_driver,
-options::OPT_fno_openmp_new_driver, true) &&
+options::OPT_no_offload_new_driver, true) &&
+  !Args.hasFlag(options::OPT_offload_new_driver,
+options::OPT_no_offload_new_driver, false) &&
   !Triple.isAMDGPU()) {
 D.Diag(diag::err_drv_unsupported_opt_for_target)
 << Args.getLastArg(options::OPT_foffload_lto,
Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -3976,17 +3976,19 @@
   // 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_no_offload_new_driver, true)) ||
+  Args.hasFlag(options::OPT_offload_new_driver,
+  options::OPT_no_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  : Inputs) {
 types::ID InputType = I.first;
 const Arg *InputArg = I.second;
@@ -4114,8 +4116,7 @@
 // Check if this Linker Job should emit a static library.
 if (ShouldEmitStaticLibrary(Args)) {
   LA = C.MakeAction(LinkerInputs, types::TY_Image);
-} else if (UseNewOffloadingDriver &&
-   C.getActiveOffloadKinds() != Action::OFK_None) {
+} else if (UseNewOffloadingDriver) {
   LA = C.MakeAction(LinkerInputs, types::TY_Image);
   LA->propagateHostOffloadInfo(C.getActiveOffloadKinds(),
/*BoundArch=*/nullptr);
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -2532,10 +2532,14 @@
   PosFlag, NegFlag, 
BothFlags<[NoArgumentUnused, HelpHidden]>>;
 def static_openmp: Flag<["-"], "static-openmp">,
   HelpText<"Use the static host OpenMP runtime while linking.">;
+def offload_new_driver : Flag<["--"], "offload-new-driver">, 
Flags<[CC1Option]>, Group,
+  HelpText<"Use the new driver for offloading compilation.">;
+def no_offload_new_driver : Flag<["--"], "no-offload-new-driver">, 
Flags<[CC1Option]>, Group,
+  HelpText<"Don't Use the new driver for offloading compilation.">;
 def fopenmp_new_driver : Flag<["-"], "fopenmp-new-driver">, 
Flags<[CC1Option]>, Group,
   HelpText<"Use the new driver for OpenMP offloading.">;
 def fno_openmp_new_driver : Flag<["-"], "fno-openmp-new-driver">, 
Flags<[CC1Option]>, Group,
-  HelpText<"Don't use the new driver for OpenMP offloading.">;
+  Alias, HelpText<"Don't use the new driver for OpenMP 
offloading.">;
 def fno_optimize_sibling_calls : Flag<["-"], "fno-optimize-sibling-calls">, 
Group, Flags<[CC1Option]>,
   HelpText<"Disable tail call optimization, keeping the call stack accurate">,
   MarshallingInfoFlag>;


Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ 

[PATCH] D123325: [Clang] Make enabling the new driver more generic

[Joseph Huber via Phabricator via cfe-commits]

jhuber6 added inline comments.



Comment at: clang/include/clang/Driver/Options.td:2535-2538
+def foffload_new_driver : Flag<["-"], "foffload-new-driver">, 
Flags<[CC1Option]>, Group,
+  HelpText<"Use the new driver for offloading compilation.">;
+def fno_offload_new_driver : Flag<["-"], "fno-offload-new-driver">, 
Flags<[CC1Option]>, Group,
+  HelpText<"Don't Use the new driver for offloading compilation.">;

tra wrote:
> jhuber6 wrote:
> > tra wrote:
> > > 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`.
> > > 
> > 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.
> 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).
> 
> 
Sure, I'll change it to just be offload.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D123325/new/

https://reviews.llvm.org/D123325

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D123325: [Clang] Make enabling the new driver more generic

[Artem Belevich via Phabricator via cfe-commits]

tra added inline comments.



Comment at: clang/include/clang/Driver/Options.td:2535-2538
+def foffload_new_driver : Flag<["-"], "foffload-new-driver">, 
Flags<[CC1Option]>, Group,
+  HelpText<"Use the new driver for offloading compilation.">;
+def fno_offload_new_driver : Flag<["-"], "fno-offload-new-driver">, 
Flags<[CC1Option]>, Group,
+  HelpText<"Don't Use the new driver for offloading compilation.">;

jhuber6 wrote:
> tra wrote:
> > 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`.
> > 
> 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.
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).




Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D123325/new/

https://reviews.llvm.org/D123325

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D123325: [Clang] Make enabling the new driver more generic

[Joseph Huber via Phabricator via cfe-commits]

jhuber6 added inline comments.



Comment at: clang/include/clang/Driver/Options.td:2535-2538
+def foffload_new_driver : Flag<["-"], "foffload-new-driver">, 
Flags<[CC1Option]>, Group,
+  HelpText<"Use the new driver for offloading compilation.">;
+def fno_offload_new_driver : Flag<["-"], "fno-offload-new-driver">, 
Flags<[CC1Option]>, Group,
+  HelpText<"Don't Use the new driver for offloading compilation.">;

tra wrote:
> 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`.
> 
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.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D123325/new/

https://reviews.llvm.org/D123325

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D123325: [Clang] Make enabling the new driver more generic

[Artem Belevich via Phabricator via cfe-commits]

tra added inline comments.



Comment at: clang/include/clang/Driver/Options.td:2535-2538
+def foffload_new_driver : Flag<["-"], "foffload-new-driver">, 
Flags<[CC1Option]>, Group,
+  HelpText<"Use the new driver for offloading compilation.">;
+def fno_offload_new_driver : Flag<["-"], "fno-offload-new-driver">, 
Flags<[CC1Option]>, Group,
+  HelpText<"Don't Use the new driver for offloading compilation.">;

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`.



Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D123325/new/

https://reviews.llvm.org/D123325

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D123325: [Clang] Make enabling the new driver more generic

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl accepted this revision.
yaxunl added a comment.
This revision is now accepted and ready to land.

LGTM. Thanks.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D123325/new/

https://reviews.llvm.org/D123325

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D123325: [Clang] Make enabling the new driver more generic

[Joseph Huber via Phabricator via cfe-commits]

jhuber6 updated this revision to Diff 425785.
jhuber6 added a comment.

Ping and update.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D123325/new/

https://reviews.llvm.org/D123325

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/ToolChains/Clang.cpp


Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -4381,8 +4381,11 @@
   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);
 
@@ -4678,7 +4681,9 @@
   // 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,
Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -3976,17 +3976,19 @@
   // 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  : Inputs) {
 types::ID InputType = I.first;
 const Arg *InputArg = I.second;
@@ -4114,8 +4116,7 @@
 // Check if this Linker Job should emit a static library.
 if (ShouldEmitStaticLibrary(Args)) {
   LA = C.MakeAction(LinkerInputs, types::TY_Image);
-} else if (UseNewOffloadingDriver &&
-   C.getActiveOffloadKinds() != Action::OFK_None) {
+} else if (UseNewOffloadingDriver) {
   LA = C.MakeAction(LinkerInputs, types::TY_Image);
   LA->propagateHostOffloadInfo(C.getActiveOffloadKinds(),
/*BoundArch=*/nullptr);
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -2532,10 +2532,14 @@
   PosFlag, NegFlag, 
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,
+  HelpText<"Use the new driver for offloading compilation.">;
+def fno_offload_new_driver : Flag<["-"], "fno-offload-new-driver">, 
Flags<[CC1Option]>, Group,
+  HelpText<"Don't Use the new driver for offloading compilation.">;
 def fopenmp_new_driver : Flag<["-"], "fopenmp-new-driver">, 
Flags<[CC1Option]>, Group,
   HelpText<"Use the new driver for OpenMP offloading.">;
 def fno_openmp_new_driver : Flag<["-"], "fno-openmp-new-driver">, 
Flags<[CC1Option]>, Group,
-  HelpText<"Don't use the new driver for OpenMP offloading.">;
+  Alias, HelpText<"Don't use the new driver for OpenMP 
offloading.">;
 def fno_optimize_sibling_calls : Flag<["-"], "fno-optimize-sibling-calls">, 
Group, Flags<[CC1Option]>,
   HelpText<"Disable tail call optimization, keeping the call stack accurate">,
   MarshallingInfoFlag>;


Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ 

[PATCH] D123325: [Clang] Make enabling the new driver more generic

[Joseph Huber via Phabricator via cfe-commits]

jhuber6 added a comment.

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.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D123325/new/

https://reviews.llvm.org/D123325

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D123325: [Clang] Make enabling the new driver more generic

[Joseph Huber via Phabricator via cfe-commits]

jhuber6 added a comment.

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.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D123325/new/

https://reviews.llvm.org/D123325

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D123325: [Clang] Make enabling the new driver more generic

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl added a comment.

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


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D123325/new/

https://reviews.llvm.org/D123325

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D123325: [Clang] Make enabling the new driver more generic

[Joseph Huber via Phabricator via cfe-commits]

jhuber6 updated this revision to Diff 423514.
jhuber6 added a comment.

Removing unnecessary change


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D123325/new/

https://reviews.llvm.org/D123325

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/ToolChains/Clang.cpp


Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -4381,8 +4381,9 @@
   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.hasArg(options::OPT_fno_openmp_new_driver)) ||
+  Args.hasArg(options::OPT_foffload_new_driver);
   bool IsUsingLTO = D.isUsingLTO(IsDeviceOffloadAction);
   auto LTOMode = D.getLTOMode(IsDeviceOffloadAction);
 
@@ -4674,6 +4675,7 @@
   // Only AMDGPU supports device-side LTO.
   if (IsDeviceOffloadAction &&
   Args.hasArg(options::OPT_fno_openmp_new_driver) &&
+  !Args.hasArg(options::OPT_foffload_new_driver) &&
   !Triple.isAMDGPU()) {
 D.Diag(diag::err_drv_unsupported_opt_for_target)
 << Args.getLastArg(options::OPT_foffload_lto,
Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -3891,16 +3891,17 @@
   // Builder to be used to build offloading actions.
   OffloadingActionBuilder OffloadBuilder(C, Args, Inputs);
 
+  bool UseNewOffloadingDriver =
+  (C.isOffloadingHostKind(Action::OFK_OpenMP) &&
+  !Args.hasArg(options::OPT_fno_openmp_new_driver)) ||
+  Args.hasArg(options::OPT_foffload_new_driver);
+
   // Construct the actions to perform.
   HeaderModulePrecompileJobAction *HeaderModuleAction = nullptr;
   ExtractAPIJobAction *ExtractAPIAction = nullptr;
   ActionList LinkerInputs;
   ActionList MergerInputs;
 
-  bool UseNewOffloadingDriver =
-  C.isOffloadingHostKind(Action::OFK_OpenMP) &&
-  !Args.hasArg(options::OPT_fno_openmp_new_driver);
-
   for (auto  : Inputs) {
 types::ID InputType = I.first;
 const Arg *InputArg = I.second;
@@ -4024,8 +4025,7 @@
 // Check if this Linker Job should emit a static library.
 if (ShouldEmitStaticLibrary(Args)) {
   LA = C.MakeAction(LinkerInputs, types::TY_Image);
-} else if (UseNewOffloadingDriver &&
-   C.getActiveOffloadKinds() != Action::OFK_None) {
+} else if (UseNewOffloadingDriver) {
   LA = C.MakeAction(LinkerInputs, types::TY_Image);
   LA->propagateHostOffloadInfo(C.getActiveOffloadKinds(),
/*BoundArch=*/nullptr);
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -2523,6 +2523,8 @@
   HelpText<"Use the new driver for OpenMP offloading.">;
 def fno_openmp_new_driver : Flag<["-"], "fno-openmp-new-driver">, 
Flags<[CC1Option]>, Group,
   HelpText<"Don't use the new driver for OpenMP offloading.">;
+def foffload_new_driver : Flag<["-"], "foffload-new-driver">, 
Flags<[CC1Option]>, Group,
+  HelpText<"Use the new driver for offloading.">;
 def fno_optimize_sibling_calls : Flag<["-"], "fno-optimize-sibling-calls">, 
Group, Flags<[CC1Option]>,
   HelpText<"Disable tail call optimization, keeping the call stack accurate">,
   MarshallingInfoFlag>;


Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -4381,8 +4381,9 @@
   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.hasArg(options::OPT_fno_openmp_new_driver)) ||
+  Args.hasArg(options::OPT_foffload_new_driver);
   bool IsUsingLTO = D.isUsingLTO(IsDeviceOffloadAction);
   auto LTOMode = D.getLTOMode(IsDeviceOffloadAction);
 
@@ -4674,6 +4675,7 @@
   // Only AMDGPU supports device-side LTO.
   if (IsDeviceOffloadAction &&
   Args.hasArg(options::OPT_fno_openmp_new_driver) &&
+  !Args.hasArg(options::OPT_foffload_new_driver) &&
   !Triple.isAMDGPU()) {
 D.Diag(diag::err_drv_unsupported_opt_for_target)
 << Args.getLastArg(options::OPT_foffload_lto,
Index: 

[PATCH] D123325: [Clang] Make enabling the new driver more generic

[Joseph Huber via Phabricator via cfe-commits]

jhuber6 updated this revision to Diff 423479.
jhuber6 added a comment.

Fix


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D123325/new/

https://reviews.llvm.org/D123325

Files:
  clang/include/clang/Driver/Compilation.h
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/ToolChains/Clang.cpp


Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -4381,8 +4381,9 @@
   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.hasArg(options::OPT_fno_openmp_new_driver)) ||
+  Args.hasArg(options::OPT_foffload_new_driver);
   bool IsUsingLTO = D.isUsingLTO(IsDeviceOffloadAction);
   auto LTOMode = D.getLTOMode(IsDeviceOffloadAction);
 
@@ -4674,6 +4675,7 @@
   // Only AMDGPU supports device-side LTO.
   if (IsDeviceOffloadAction &&
   Args.hasArg(options::OPT_fno_openmp_new_driver) &&
+  !Args.hasArg(options::OPT_foffload_new_driver) &&
   !Triple.isAMDGPU()) {
 D.Diag(diag::err_drv_unsupported_opt_for_target)
 << Args.getLastArg(options::OPT_foffload_lto,
Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -3891,16 +3891,17 @@
   // Builder to be used to build offloading actions.
   OffloadingActionBuilder OffloadBuilder(C, Args, Inputs);
 
+  bool UseNewOffloadingDriver =
+  (C.isOffloadingHostKind(Action::OFK_OpenMP) &&
+  !Args.hasArg(options::OPT_fno_openmp_new_driver)) ||
+  Args.hasArg(options::OPT_foffload_new_driver);
+
   // Construct the actions to perform.
   HeaderModulePrecompileJobAction *HeaderModuleAction = nullptr;
   ExtractAPIJobAction *ExtractAPIAction = nullptr;
   ActionList LinkerInputs;
   ActionList MergerInputs;
 
-  bool UseNewOffloadingDriver =
-  C.isOffloadingHostKind(Action::OFK_OpenMP) &&
-  !Args.hasArg(options::OPT_fno_openmp_new_driver);
-
   for (auto  : Inputs) {
 types::ID InputType = I.first;
 const Arg *InputArg = I.second;
@@ -4024,8 +4025,7 @@
 // Check if this Linker Job should emit a static library.
 if (ShouldEmitStaticLibrary(Args)) {
   LA = C.MakeAction(LinkerInputs, types::TY_Image);
-} else if (UseNewOffloadingDriver &&
-   C.getActiveOffloadKinds() != Action::OFK_None) {
+} else if (UseNewOffloadingDriver) {
   LA = C.MakeAction(LinkerInputs, types::TY_Image);
   LA->propagateHostOffloadInfo(C.getActiveOffloadKinds(),
/*BoundArch=*/nullptr);
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -2523,6 +2523,8 @@
   HelpText<"Use the new driver for OpenMP offloading.">;
 def fno_openmp_new_driver : Flag<["-"], "fno-openmp-new-driver">, 
Flags<[CC1Option]>, Group,
   HelpText<"Don't use the new driver for OpenMP offloading.">;
+def foffload_new_driver : Flag<["-"], "foffload-new-driver">, 
Flags<[CC1Option]>, Group,
+  HelpText<"Use the new driver for offloading.">;
 def fno_optimize_sibling_calls : Flag<["-"], "fno-optimize-sibling-calls">, 
Group, Flags<[CC1Option]>,
   HelpText<"Disable tail call optimization, keeping the call stack accurate">,
   MarshallingInfoFlag>;
Index: clang/include/clang/Driver/Compilation.h
===
--- clang/include/clang/Driver/Compilation.h
+++ clang/include/clang/Driver/Compilation.h
@@ -139,7 +139,7 @@
 
   const ToolChain () const { return DefaultToolChain; }
 
-  unsigned isOffloadingHostKind(Action::OffloadKind Kind) const {
+  unsigned isOffloadingHostKind(unsigned int Kind) const {
 return ActiveOffloadMask & Kind;
   }
 


Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -4381,8 +4381,9 @@
   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.hasArg(options::OPT_fno_openmp_new_driver)) ||
+  Args.hasArg(options::OPT_foffload_new_driver);
   bool IsUsingLTO = 

[PATCH] D123325: [Clang] Make enabling the new driver more generic

[Joseph Huber via Phabricator via cfe-commits]

jhuber6 updated this revision to Diff 423459.
jhuber6 added a comment.

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


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D123325/new/

https://reviews.llvm.org/D123325

Files:
  clang/include/clang/Driver/Compilation.h
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/ToolChains/Clang.cpp


Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -4382,7 +4382,8 @@
  JA.isDeviceOffloading(Action::OFK_Host));
   bool IsHostOffloadingAction =
   JA.isHostOffloading(C.getActiveOffloadKinds()) &&
-  Args.hasArg(options::OPT_fopenmp_new_driver);
+  (Args.hasArg(options::OPT_fopenmp_new_driver) ||
+   Args.hasArg(options::OPT_foffload_new_driver));
   bool IsUsingLTO = D.isUsingLTO(IsDeviceOffloadAction);
   auto LTOMode = D.getLTOMode(IsDeviceOffloadAction);
 
@@ -4674,6 +4675,7 @@
   // Only AMDGPU supports device-side LTO.
   if (IsDeviceOffloadAction &&
   Args.hasArg(options::OPT_fno_openmp_new_driver) &&
+  !Args.hasArg(options::OPT_foffload_new_driver) &&
   !Triple.isAMDGPU()) {
 D.Diag(diag::err_drv_unsupported_opt_for_target)
 << Args.getLastArg(options::OPT_foffload_lto,
Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -3897,9 +3897,12 @@
   ActionList LinkerInputs;
   ActionList MergerInputs;
 
+  // We should use the new offloading driver for OpenMP offloading unless
+  // explicitly disabled.
   bool UseNewOffloadingDriver =
-  C.isOffloadingHostKind(Action::OFK_OpenMP) &&
-  !Args.hasArg(options::OPT_fno_openmp_new_driver);
+  (C.isOffloadingHostKind(Action::OFK_OpenMP) &&
+   !Args.hasArg(options::OPT_fno_openmp_new_driver)) ||
+  Args.hasArg(options::OPT_foffload_new_driver);
 
   for (auto  : Inputs) {
 types::ID InputType = I.first;
@@ -4024,8 +4027,7 @@
 // Check if this Linker Job should emit a static library.
 if (ShouldEmitStaticLibrary(Args)) {
   LA = C.MakeAction(LinkerInputs, types::TY_Image);
-} else if (UseNewOffloadingDriver &&
-   C.getActiveOffloadKinds() != Action::OFK_None) {
+} else if (UseNewOffloadingDriver) {
   LA = C.MakeAction(LinkerInputs, types::TY_Image);
   LA->propagateHostOffloadInfo(C.getActiveOffloadKinds(),
/*BoundArch=*/nullptr);
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -2523,6 +2523,8 @@
   HelpText<"Use the new driver for OpenMP offloading.">;
 def fno_openmp_new_driver : Flag<["-"], "fno-openmp-new-driver">, 
Flags<[CC1Option]>, Group,
   HelpText<"Don't use the new driver for OpenMP offloading.">;
+def foffload_new_driver : Flag<["-"], "foffload-new-driver">, 
Flags<[CC1Option]>, Group,
+  HelpText<"Use the new driver for offloading.">;
 def fno_optimize_sibling_calls : Flag<["-"], "fno-optimize-sibling-calls">, 
Group, Flags<[CC1Option]>,
   HelpText<"Disable tail call optimization, keeping the call stack accurate">,
   MarshallingInfoFlag>;
Index: clang/include/clang/Driver/Compilation.h
===
--- clang/include/clang/Driver/Compilation.h
+++ clang/include/clang/Driver/Compilation.h
@@ -139,7 +139,7 @@
 
   const ToolChain () const { return DefaultToolChain; }
 
-  unsigned isOffloadingHostKind(Action::OffloadKind Kind) const {
+  unsigned isOffloadingHostKind(unsigned int Kind) const {
 return ActiveOffloadMask & Kind;
   }
 


Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -4382,7 +4382,8 @@
  JA.isDeviceOffloading(Action::OFK_Host));
   bool IsHostOffloadingAction =
   JA.isHostOffloading(C.getActiveOffloadKinds()) &&
-  Args.hasArg(options::OPT_fopenmp_new_driver);
+  (Args.hasArg(options::OPT_fopenmp_new_driver) ||
+   Args.hasArg(options::OPT_foffload_new_driver));
   bool IsUsingLTO = D.isUsingLTO(IsDeviceOffloadAction);
   auto LTOMode = D.getLTOMode(IsDeviceOffloadAction);
 
@@ -4674,6 +4675,7 @@
   // Only AMDGPU supports device-side LTO.
   if (IsDeviceOffloadAction &&
   Args.hasArg(options::OPT_fno_openmp_new_driver) &&
+  !Args.hasArg(options::OPT_foffload_new_driver) &&
   !Triple.isAMDGPU()) {
 D.Diag(diag::err_drv_unsupported_opt_for_target)
 << 

[PATCH] D123325: [Clang] Make enabling the new driver more generic

[Joseph Huber via Phabricator via cfe-commits]

jhuber6 added inline comments.



Comment at: clang/lib/Driver/Driver.cpp:3885-3888
+  bool UseNewOffloadingDriver =
+  C.isOffloadingHostKind(C.getActiveOffloadKinds()) &&
+  (Args.hasArg(options::OPT_foffload_new_driver) ||
+   Args.hasArg(options::OPT_fopenmp_new_driver));

yaxunl wrote:
> 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?
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.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D123325/new/

https://reviews.llvm.org/D123325

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D123325: [Clang] Make enabling the new driver more generic

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl added inline comments.



Comment at: clang/lib/Driver/Driver.cpp:3885-3888
+  bool UseNewOffloadingDriver =
+  C.isOffloadingHostKind(C.getActiveOffloadKinds()) &&
+  (Args.hasArg(options::OPT_foffload_new_driver) ||
+   Args.hasArg(options::OPT_fopenmp_new_driver));

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?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D123325/new/

https://reviews.llvm.org/D123325

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D123325: [Clang] Make enabling the new driver more generic

[Joseph Huber via Phabricator via cfe-commits]

jhuber6 added a comment.

ping


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D123325/new/

https://reviews.llvm.org/D123325

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D123325: [Clang] Make enabling the new driver more generic

[Joseph Huber via Phabricator via cfe-commits]

jhuber6 updated this revision to Diff 421607.
jhuber6 added a comment.

Rebase.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D123325/new/

https://reviews.llvm.org/D123325

Files:
  clang/include/clang/Driver/Compilation.h
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/ToolChains/Clang.cpp

Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -4400,7 +4400,8 @@
  JA.isDeviceOffloading(Action::OFK_Host));
   bool IsHostOffloadingAction =
   JA.isHostOffloading(C.getActiveOffloadKinds()) &&
-  Args.hasArg(options::OPT_fopenmp_new_driver);
+  (Args.hasArg(options::OPT_fopenmp_new_driver) ||
+   Args.hasArg(options::OPT_foffload_new_driver));
   bool IsUsingLTO = D.isUsingLTO(IsDeviceOffloadAction);
   auto LTOMode = D.getLTOMode(IsDeviceOffloadAction);
 
@@ -4688,7 +4689,8 @@
 if (JA.getType() == types::TY_LLVM_BC)
   CmdArgs.push_back("-emit-llvm-uselists");
 
-if (IsUsingLTO && !Args.hasArg(options::OPT_fopenmp_new_driver)) {
+if (IsUsingLTO && !(Args.hasArg(options::OPT_fopenmp_new_driver) ||
+Args.hasArg(options::OPT_foffload_new_driver))) {
   // Only AMDGPU supports device-side LTO.
   if (IsDeviceOffloadAction && !Triple.isAMDGPU()) {
 D.Diag(diag::err_drv_unsupported_opt_for_target)
Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -3882,6 +3882,11 @@
   // Builder to be used to build offloading actions.
   OffloadingActionBuilder OffloadBuilder(C, Args, Inputs);
 
+  bool UseNewOffloadingDriver =
+  C.isOffloadingHostKind(C.getActiveOffloadKinds()) &&
+  (Args.hasArg(options::OPT_foffload_new_driver) ||
+   Args.hasArg(options::OPT_fopenmp_new_driver));
+
   // Construct the actions to perform.
   HeaderModulePrecompileJobAction *HeaderModuleAction = nullptr;
   ExtractAPIJobAction *ExtractAPIAction = nullptr;
@@ -3903,14 +3908,14 @@
 
 // Use the current host action in any of the offloading actions, if
 // required.
-if (!Args.hasArg(options::OPT_fopenmp_new_driver))
+if (!UseNewOffloadingDriver)
   if (OffloadBuilder.addHostDependenceToDeviceActions(Current, InputArg))
 break;
 
 for (phases::ID Phase : PL) {
 
   // Add any offload action the host action depends on.
-  if (!Args.hasArg(options::OPT_fopenmp_new_driver))
+  if (!UseNewOffloadingDriver)
 Current = OffloadBuilder.addDeviceDependencesToHostAction(
 Current, InputArg, Phase, PL.back(), FullPL);
   if (!Current)
@@ -3953,7 +3958,7 @@
 
   // Try to build the offloading actions and add the result as a dependency
   // to the host.
-  if (Args.hasArg(options::OPT_fopenmp_new_driver))
+  if (UseNewOffloadingDriver)
 Current = BuildOffloadingActions(C, Args, I, Current);
 
   // FIXME: Should we include any prior module file outputs as inputs of
@@ -3975,7 +3980,7 @@
 
   // Use the current host action in any of the offloading actions, if
   // required.
-  if (!Args.hasArg(options::OPT_fopenmp_new_driver))
+  if (!UseNewOffloadingDriver)
 if (OffloadBuilder.addHostDependenceToDeviceActions(Current, InputArg))
   break;
 
@@ -3988,7 +3993,7 @@
   Actions.push_back(Current);
 
 // Add any top level actions generated for offloading.
-if (!Args.hasArg(options::OPT_fopenmp_new_driver))
+if (!UseNewOffloadingDriver)
   OffloadBuilder.appendTopLevelActions(Actions, Current, InputArg);
 else if (Current)
   Current->propagateHostOffloadInfo(C.getActiveOffloadKinds(),
@@ -4004,14 +4009,14 @@
   }
 
   if (!LinkerInputs.empty()) {
-if (!Args.hasArg(options::OPT_fopenmp_new_driver))
+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(LinkerInputs, types::TY_Image);
-} else if (Args.hasArg(options::OPT_fopenmp_new_driver) &&
+} else if (UseNewOffloadingDriver &&
C.getActiveOffloadKinds() != Action::OFK_None) {
   LA = C.MakeAction(LinkerInputs, types::TY_Image);
   LA->propagateHostOffloadInfo(C.getActiveOffloadKinds(),
@@ -4019,7 +4024,7 @@
 } else {
   LA = C.MakeAction(LinkerInputs, types::TY_Image);
 }
-if (!Args.hasArg(options::OPT_fopenmp_new_driver))
+if (!UseNewOffloadingDriver)
   LA = OffloadBuilder.processHostLinkAction(LA);
 Actions.push_back(LA);
   }
Index: clang/include/clang/Driver/Options.td

[PATCH] D123325: [Clang] Make enabling the new driver more generic

[Joseph Huber via Phabricator via cfe-commits]

jhuber6 updated this revision to Diff 421595.
jhuber6 added a comment.

Fix misplaced logic symbol that broke tests.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D123325/new/

https://reviews.llvm.org/D123325

Files:
  clang/include/clang/Driver/Compilation.h
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/ToolChains/Clang.cpp

Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -4400,7 +4400,8 @@
  JA.isDeviceOffloading(Action::OFK_Host));
   bool IsHostOffloadingAction =
   JA.isHostOffloading(C.getActiveOffloadKinds()) &&
-  Args.hasArg(options::OPT_fopenmp_new_driver);
+  (Args.hasArg(options::OPT_fopenmp_new_driver) ||
+   Args.hasArg(options::OPT_foffload_new_driver));
   bool IsUsingLTO = D.isUsingLTO(IsDeviceOffloadAction);
   auto LTOMode = D.getLTOMode(IsDeviceOffloadAction);
 
@@ -4688,7 +4689,8 @@
 if (JA.getType() == types::TY_LLVM_BC)
   CmdArgs.push_back("-emit-llvm-uselists");
 
-if (IsUsingLTO && !Args.hasArg(options::OPT_fopenmp_new_driver)) {
+if (IsUsingLTO && !(Args.hasArg(options::OPT_fopenmp_new_driver) ||
+Args.hasArg(options::OPT_foffload_new_driver))) {
   // Only AMDGPU supports device-side LTO.
   if (IsDeviceOffloadAction && !Triple.isAMDGPU()) {
 D.Diag(diag::err_drv_unsupported_opt_for_target)
Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -3882,6 +3882,11 @@
   // Builder to be used to build offloading actions.
   OffloadingActionBuilder OffloadBuilder(C, Args, Inputs);
 
+  bool UseNewOffloadingDriver =
+  C.isOffloadingHostKind(C.getActiveOffloadKinds()) &&
+  (Args.hasArg(options::OPT_foffload_new_driver) ||
+   Args.hasArg(options::OPT_fopenmp_new_driver));
+
   // Construct the actions to perform.
   HeaderModulePrecompileJobAction *HeaderModuleAction = nullptr;
   ExtractAPIJobAction *ExtractAPIAction = nullptr;
@@ -3903,14 +3908,14 @@
 
 // Use the current host action in any of the offloading actions, if
 // required.
-if (!Args.hasArg(options::OPT_fopenmp_new_driver))
+if (!UseNewOffloadingDriver)
   if (OffloadBuilder.addHostDependenceToDeviceActions(Current, InputArg))
 break;
 
 for (phases::ID Phase : PL) {
 
   // Add any offload action the host action depends on.
-  if (!Args.hasArg(options::OPT_fopenmp_new_driver))
+  if (!UseNewOffloadingDriver)
 Current = OffloadBuilder.addDeviceDependencesToHostAction(
 Current, InputArg, Phase, PL.back(), FullPL);
   if (!Current)
@@ -3953,7 +3958,7 @@
 
   // Try to build the offloading actions and add the result as a dependency
   // to the host.
-  if (Args.hasArg(options::OPT_fopenmp_new_driver))
+  if (UseNewOffloadingDriver)
 Current = BuildOffloadingActions(C, Args, I, Current);
 
   // FIXME: Should we include any prior module file outputs as inputs of
@@ -3975,7 +3980,7 @@
 
   // Use the current host action in any of the offloading actions, if
   // required.
-  if (!Args.hasArg(options::OPT_fopenmp_new_driver))
+  if (!UseNewOffloadingDriver)
 if (OffloadBuilder.addHostDependenceToDeviceActions(Current, InputArg))
   break;
 
@@ -3988,7 +3993,7 @@
   Actions.push_back(Current);
 
 // Add any top level actions generated for offloading.
-if (!Args.hasArg(options::OPT_fopenmp_new_driver))
+if (!UseNewOffloadingDriver)
   OffloadBuilder.appendTopLevelActions(Actions, Current, InputArg);
 else if (Current)
   Current->propagateHostOffloadInfo(C.getActiveOffloadKinds(),
@@ -4004,14 +4009,14 @@
   }
 
   if (!LinkerInputs.empty()) {
-if (!Args.hasArg(options::OPT_fopenmp_new_driver))
+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(LinkerInputs, types::TY_Image);
-} else if (Args.hasArg(options::OPT_fopenmp_new_driver) &&
+} else if (UseNewOffloadingDriver &&
C.getActiveOffloadKinds() != Action::OFK_None) {
   LA = C.MakeAction(LinkerInputs, types::TY_Image);
   LA->propagateHostOffloadInfo(C.getActiveOffloadKinds(),
@@ -4019,7 +4024,7 @@
 } else {
   LA = C.MakeAction(LinkerInputs, types::TY_Image);
 }
-if (!Args.hasArg(options::OPT_fopenmp_new_driver))
+if (!UseNewOffloadingDriver)
   LA = OffloadBuilder.processHostLinkAction(LA);
 Actions.push_back(LA);
   }
Index: 

[PATCH] D123325: [Clang] Make enabling the new driver more generic

[Joseph Huber via Phabricator via cfe-commits]

jhuber6 created this revision.
jhuber6 added reviewers: JonChesterfield, jdoerfert, yaxunl, tra.
Herald added a project: All.
jhuber6 requested review of this revision.
Herald added subscribers: cfe-commits, sstefan1, MaskRay.
Herald added a project: clang.

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.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D123325

Files:
  clang/include/clang/Driver/Compilation.h
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/ToolChains/Clang.cpp

Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -4395,7 +4395,8 @@
  JA.isDeviceOffloading(Action::OFK_Host));
   bool IsHostOffloadingAction =
   JA.isHostOffloading(C.getActiveOffloadKinds()) &&
-  Args.hasArg(options::OPT_fopenmp_new_driver);
+  (Args.hasArg(options::OPT_fopenmp_new_driver) ||
+   Args.hasArg(options::OPT_foffload_new_driver));
   bool IsUsingLTO = D.isUsingLTO(IsDeviceOffloadAction);
   auto LTOMode = D.getLTOMode(IsDeviceOffloadAction);
 
@@ -4683,7 +4684,8 @@
 if (JA.getType() == types::TY_LLVM_BC)
   CmdArgs.push_back("-emit-llvm-uselists");
 
-if (IsUsingLTO && !Args.hasArg(options::OPT_fopenmp_new_driver)) {
+if (IsUsingLTO && !(Args.hasArg(options::OPT_fopenmp_new_driver) ||
+!Args.hasArg(options::OPT_foffload_new_driver))) {
   // Only AMDGPU supports device-side LTO.
   if (IsDeviceOffloadAction && !Triple.isAMDGPU()) {
 D.Diag(diag::err_drv_unsupported_opt_for_target)
Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -3881,6 +3881,11 @@
   // Builder to be used to build offloading actions.
   OffloadingActionBuilder OffloadBuilder(C, Args, Inputs);
 
+  bool UseNewOffloadingDriver =
+  C.isOffloadingHostKind(C.getActiveOffloadKinds()) &&
+  (Args.hasArg(options::OPT_foffload_new_driver) ||
+   Args.hasArg(options::OPT_fopenmp_new_driver));
+
   // Construct the actions to perform.
   HeaderModulePrecompileJobAction *HeaderModuleAction = nullptr;
   ExtractAPIJobAction *ExtractAPIAction = nullptr;
@@ -3902,14 +3907,14 @@
 
 // Use the current host action in any of the offloading actions, if
 // required.
-if (!Args.hasArg(options::OPT_fopenmp_new_driver))
+if (!UseNewOffloadingDriver)
   if (OffloadBuilder.addHostDependenceToDeviceActions(Current, InputArg))
 break;
 
 for (phases::ID Phase : PL) {
 
   // Add any offload action the host action depends on.
-  if (!Args.hasArg(options::OPT_fopenmp_new_driver))
+  if (!UseNewOffloadingDriver)
 Current = OffloadBuilder.addDeviceDependencesToHostAction(
 Current, InputArg, Phase, PL.back(), FullPL);
   if (!Current)
@@ -3952,7 +3957,7 @@
 
   // Try to build the offloading actions and add the result as a dependency
   // to the host.
-  if (Args.hasArg(options::OPT_fopenmp_new_driver))
+  if (UseNewOffloadingDriver)
 Current = BuildOffloadingActions(C, Args, I, Current);
 
   // FIXME: Should we include any prior module file outputs as inputs of
@@ -3974,7 +3979,7 @@
 
   // Use the current host action in any of the offloading actions, if
   // required.
-  if (!Args.hasArg(options::OPT_fopenmp_new_driver))
+  if (!UseNewOffloadingDriver)
 if (OffloadBuilder.addHostDependenceToDeviceActions(Current, InputArg))
   break;
 
@@ -3987,7 +3992,7 @@
   Actions.push_back(Current);
 
 // Add any top level actions generated for offloading.
-if (!Args.hasArg(options::OPT_fopenmp_new_driver))
+if (!UseNewOffloadingDriver)
   OffloadBuilder.appendTopLevelActions(Actions, Current, InputArg);
 else if (Current)
   Current->propagateHostOffloadInfo(C.getActiveOffloadKinds(),
@@ -4003,14 +4008,14 @@
   }
 
   if (!LinkerInputs.empty()) {
-if (!Args.hasArg(options::OPT_fopenmp_new_driver))
+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(LinkerInputs, types::TY_Image);
-} else if (Args.hasArg(options::OPT_fopenmp_new_driver) &&
+} else if (UseNewOffloadingDriver &&
C.getActiveOffloadKinds() != Action::OFK_None) {
   LA = C.MakeAction(LinkerInputs, types::TY_Image);