tra accepted this revision.
tra added a subscriber: tejohnson.
tra added a comment.
This revision is now accepted and ready to land.
LGTM in general. Please give LTO folks some time to chime in case they have any
feedback.
@tejohnson: Just a FYI that we're tinkering with LTO on GPUs here.
================
Comment at: clang/include/clang/Driver/Options.td:1904-1907
+def foffload_lto : Flag<["-"], "foffload-lto">, Flags<[CoreOption]>,
Group<f_Group>,
+ HelpText<"Enable LTO in 'full' mode for offload compilation">;
+def fno_offload_lto : Flag<["-"], "fno-offload-lto">, Flags<[CoreOption]>,
Group<f_Group>,
+ HelpText<"Disable LTO mode (default) for offload compilation">;
----------------
Should it be `BoolFOption` ?
================
Comment at: clang/lib/Driver/Driver.cpp:623
+ llvm::errs() << LTOName << '\n';
LTOMode = llvm::StringSwitch<LTOKind>(LTOName)
----------------
Leftover debug printout?
================
Comment at: clang/lib/Driver/ToolChains/Clang.cpp:4420
- // Device-side jobs do not support LTO.
- bool isDeviceOffloadAction = !(JA.isDeviceOffloading(Action::OFK_None) ||
- JA.isDeviceOffloading(Action::OFK_Host));
-
- if (D.isUsingLTO() && !isDeviceOffloadAction) {
+ // Device-side jobs do not support LTO, except AMDGPU
+ if (IsUsingLTO && (!IsDeviceOffloadAction || Triple.isAMDGPU())) {
----------------
Nit: rephrase it as `Only AMDGPU supports device-side LTO` ?
================
Comment at: llvm/test/Transforms/FunctionImport/noinline.ll:4
+; RUN: opt -module-summary %p/Inputs/noinline.ll -o %t2.bc
+; RUN: llvm-lto -thinlto -o %t3 %t.bc %t2.bc
+
----------------
I'd add a meaningful suffix to the binaries we'll use to run the checks on. E.g
`%t3` -> `%t.lto.bc`, `%t2` -> `%t.inputs.noinline.bc`, `%t` -> `%t.main.bc`.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D99683/new/
https://reviews.llvm.org/D99683
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
