[PATCH] D123804: [WIP][clang][lld] A Unified LTO Bitcode Frontend

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added a comment.

Thank you for the update.

IIUC `-flto=full -funified-lto`, `-flto -funified-lto`, and `-flto=thin -funified-lto` are identical.
`-flto=full -funified-lto` may be confusing as we treat as it `-flto=thin` and use `buildThinLTOPreLinkDefaultPipeline`.

In the absence of `-flto`, `-funified-lto` seems a no-op (I do not see a test).

I wonder whether we should change `-funified-lto` to become `-flto=unified` instead.
(For the lld option, I guess `--lto-mode={thin,full}` may be clearer than `--lto={thin,full}`, but I don't have a strong opinion... )



================
Comment at: clang/lib/Frontend/CompilerInvocation.cpp:1779
       Diags.Report(diag::err_drv_invalid_value) << A->getAsString(Args) << S;
+    if (Arg *A = Args.getLastArg(OPT_funified_lto))
+      Opts.PrepareForThinLTO = true;
----------------
`hasArg` if you don't use `A`


================
Comment at: clang/test/Driver/lto-unit.c:9
 // UNIT: "-flto-unit"
+// NOUNIT-NOT: "-flto-unit"
----------------
Is `NOUNIT` unused?


================
Comment at: clang/test/Driver/unified-lto.c:5
+// UNIT: "-flto-unit"
+// NOUNIT-NOT: "-flto-unit"
+
----------------
Is `NOUNIT-NOT` used?


================
Comment at: clang/test/Driver/unified-lto.c:9
+// RUN: %clang --target=x86_64-pc-linux-gnu -fwhole-program-vtables -fno-unified-lto -### %s 2>&1 | FileCheck --check-prefix=WARN %s
+// WARN: invalid argument '-fwhole-program-vtables' only allowed with '-flto'
----------------
This test may be better placed in whole-program-vtables.c where this diagnostic is tested.



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

https://reviews.llvm.org/D123804