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

[Fangrui Song via Phabricator via cfe-commits]

MaskRay accepted this revision.
MaskRay added a comment.

There is a `-fno-fat-lto-objects` issue, but otherwise looks good after some nits are addressed. Thanks!



================
Comment at: clang/docs/ReleaseNotes.rst:250
 
+- `-ffat-lto-objects` can now be used to emit object files with both object
+  code and bitcode. Previously this flag was ignored for GCC compatibility.
----------------



================
Comment at: clang/include/clang/Driver/Options.td:2311
   MarshallingInfoString<CodeGenOpts<"ThinLinkBitcodeFile">>;
+defm fat_lto_objects : BoolFOption<"fat-lto-objects",
+  CodeGenOpts<"FatLTO">, DefaultFalse,
----------------
We just need the pos flag for CC1Option, so use `OptInCC1FFlag`


================
Comment at: clang/lib/Driver/ToolChains/Clang.cpp:7297
 
+  if (IsUsingLTO && Args.getLastArg(options::OPT_ffat_lto_objects)) {
+    assert(LTOMode == LTOK_Full || LTOMode == LTOK_Thin);
----------------
This does not consider the negative option.

```
if (IsUsingLTO) {
  if (Arg *A = Args.getLastArg(options::OPT_ffat_lto_objects, options::OPT_fno_fat_lto_objects)) {
    if (A->getOption().matches(options::OPT_ffat_lto_objects)) {
      CmdArgs.push_back(...)
      A->render(Args, CmdArgs);
    }
  }
}
```

or move `getLastArg` outside of `if (IsUsingLTO)` to avoid -Wunused-command-line-argument in the absence of `-flto=`.

This code block probably should be moved before `fglobal_isel` to be closer to other LTO code.


================
Comment at: clang/test/CodeGen/embed-lto-fatlto.c:1
+// RUN: %clang -cc1 -triple x86_64-unknown-linux-gnu -S -flto=full -ffat-lto-objects -fsplit-lto-unit -emit-llvm < %s  | FileCheck %s --check-prefixes=FULL,SPLIT
+// RUN: %clang -cc1 -triple x86_64-unknown-linux-gnu -S -flto=full -ffat-lto-objects -emit-llvm < %s  | FileCheck %s --check-prefixes=FULL,SPLIT
----------------
Remove `-S`. In CC1, `-emit-llvm` wins.


================
Comment at: clang/test/CodeGen/embed-lto-fatlto.c:8
+/// Check that the ThinLTO metadata is only set false for full LTO
+// FULL: !{{[0-9]+}} = !{i32 1, !"ThinLTO", i32 0}
+// THIN-NOT: !{{[0-9]+}} = !{i32 1, !"ThinLTO", i32 0}
----------------



================
Comment at: clang/test/CodeGen/embed-lto-fatlto.c:11
+
+/// Be sure we enable split LTO unints correctly under -ffat-lto-objects
+// SPLIT: !{{[0-9]+}} = !{i32 1, !"EnableSplitLTOUnit", i32 1}
----------------



================
Comment at: clang/test/Driver/fatlto-objects.c:1
+// REQUIRES: x86_64-linux
+// RUN: %clang --target=x86_64-unknown-linux-gnu -flto -ffat-lto-objects -fintegrated-as -### %s -c 2>&1 | FileCheck %s -check-prefix=CHECK-CC
----------------
Delete the `REQUIRES`. The driver code should be portable as long as we only use `-###`.


================
Comment at: clang/test/Driver/fatlto-objects.c:1
+// REQUIRES: x86_64-linux
+// RUN: %clang --target=x86_64-unknown-linux-gnu -flto -ffat-lto-objects -fintegrated-as -### %s -c 2>&1 | FileCheck %s -check-prefix=CHECK-CC
----------------
MaskRay wrote:
> Delete the `REQUIRES`. The driver code should be portable as long as we only use `-###`.
`fat-lto-objects.c` or `ffat-lto-objects.c`


================
Comment at: clang/test/Driver/fatlto-objects.c:2
+// REQUIRES: x86_64-linux
+// RUN: %clang --target=x86_64-unknown-linux-gnu -flto -ffat-lto-objects -fintegrated-as -### %s -c 2>&1 | FileCheck %s -check-prefix=CHECK-CC
+// CHECK-CC: -cc1
----------------
Delete `-fintegrated-as`. It's the default.


================
Comment at: clang/test/Driver/fatlto-objects.c:4
+// CHECK-CC: -cc1
+// CHECK-CC: -emit-obj
+// CHECK-CC: -ffat-lto-objects
----------------
Use `-SAME:` whenever applicable.


================
Comment at: clang/test/Driver/fatlto-objects.c:7
+
+// RUN: %clang --target=x86_64-unknown-linux-gnu -ffat-lto-objects -fintegrated-as -### %s -c 2>&1 | FileCheck %s -check-prefix=CHECK-CC-NOLTO
+// CHECK-CC-NOLTO: -cc1
----------------
You may test that `-ffat-lto-objects` in the absence of `-flto=` gives a `warning: argument unused during compilation: '-ffat-lto-objects'` warning


================
Comment at: clang/test/Driver/fatlto-objects.c:18
+// RUN:   -fuse-ld=lld -fno-lto -ffat-lto-objects -### 2>&1 | FileCheck --check-prefix=NOLTO %s
+// LTO: -fat-lto-objects
+// NOLTO-NOT: -fat-lto-objects
----------------



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146777