[PATCH] D53891: [LTO] Add option to enable LTOUnit splitting, and disable unless needed
Teresa Johnson via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Wed Dec 19 08:51:15 PST 2018
tejohnson marked 2 inline comments as done.
tejohnson added inline comments.
================
Comment at: lib/CodeGen/BackendUtil.cpp:831
+ *OS, CodeGenOpts.EmitLLVMUseLists, EmitLTOSummary,
+ /*EmitModuleHash=*/false));
}
----------------
pcc wrote:
> Why add this argument here (and below)?
I think this was leftover from an earlier version where I had added a new parameter. Will revert this change (here and below).
================
Comment at: lib/Driver/ToolChains/Clang.cpp:5112
+ bool EnableSplitLTOUnit = Args.hasFlag(
+ options::OPT_fsplit_lto_unit, options::OPT_fno_split_lto_unit, false);
+ if (EnableSplitLTOUnit || WholeProgramVTables || Sanitize.needsLTO()) {
----------------
pcc wrote:
> Should this default to `WholeProgramVTables || Sanitize.needsLTO()` and emit an error if the user passes the (for now) unsupported combinations `-fno-split-lto-unit -fwhole-program-vtables` or `-fno-split-lto-unit -fsanitize=cfi`?
I think the code below needs to stay as is to allow -fsplit-lto-unit to also enable the splitting even when the other options aren't on (not sure when that would be used outside of testing, but I'm assuming we want a way to force that on).
But yes I think it makes sense to emit an error on those combinations (when my follow on patches go in then we would remove the error with -fno-split-lto-unit -fwhole-program-vtables).
Repository:
rC Clang
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D53891/new/
https://reviews.llvm.org/D53891
More information about the cfe-commits
mailing list