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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits