[PATCH] D146777: [clang] Preliminary fat-lto-object support

[Paul Kirth via Phabricator via cfe-commits]

paulkirth added inline comments.


================
Comment at: clang/lib/CodeGen/BackendUtil.cpp:1065
+  if (CodeGenOpts.FatLTO) {
+    // Set EnableSplitLTOUnit, since the config above won't
+    if (!TheModule->getModuleFlag("EnableSplitLTOUnit"))
----------------
tejohnson wrote:
> Can you expand the comment a bit - specifically why it won't end up in the above handling? I assume the Action type is different for FatLTO?
yes, we're not either of the above Action types. I'll expand that description to include more context, but maybe it should also be an `else if(...)`to make this code's relationship with the above more clear?



================
Comment at: clang/lib/Driver/ToolChains/CommonArgs.cpp:622
     CmdArgs.push_back(Args.MakeArgString(Twine(PluginPrefix) + Plugin));
+  }else{
+    // For LLD we need to enable fat object support
----------------
tejohnson wrote:
> nit: missing spaces
TY. Looks like it needs a `git clang-format`. I'll be sure to run that again when I address the other comments.


================
Comment at: clang/lib/Driver/ToolChains/CommonArgs.cpp:625
+    if (Args.hasArg(options::OPT_ffat_lto_objects))
+      CmdArgs.push_back("-fat-lto-objects");
   }
----------------
tejohnson wrote:
> Needs a test
Noted.


================
Comment at: clang/test/CodeGen/embed-lto-fatlto.c:2
+// RUN: %clang -cc1 -triple x86_64-unknown-linux-gnu -S -flto=full -ffat-lto-objects -emit-llvm < %s  | FileCheck %s
+// RUN: %clang -cc1 -triple x86_64-unknown-linux-gnu -S -flto=full -ffat-lto-objects -emit-llvm < %s  | FileCheck %s
+//
----------------
tejohnson wrote:
> Can you also test -flto=thin, with and without LTO splitting enabled?
will do.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146777