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

[Matthew Voss via Phabricator via llvm-commits]

ormris added a comment.

> IIUC -flto=full -funified-lto, -flto -funified-lto, and -flto=thin -funified-lto are identical.

They're not completely identical. They change the flag passed to the linker when the linker is invoked by the compiler. I think it's worth preserving that workflow. It's used fairly often for manually compiling a project (e.g. `clang -flto=thin -funified-lto hello.cpp`), but also by CMake.

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

Good point. Fixed.

> For the lld option, I guess --lto-mode={thin,full}

Maybe.... I was trying to mirror `-flto` and reduce the amount of typing required.



================
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;
----------------
MaskRay wrote:
> `hasArg` if you don't use `A`
Fixed.


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


================
Comment at: clang/test/Driver/unified-lto.c:5
+// UNIT: "-flto-unit"
+// NOUNIT-NOT: "-flto-unit"
+
----------------
MaskRay wrote:
> Is `NOUNIT-NOT` used?
No, but it should be. Fixed.


================
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'
----------------
MaskRay wrote:
> This test may be better placed in whole-program-vtables.c where this diagnostic is tested.
> 
Fixed


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

https://reviews.llvm.org/D123804