[PATCH] D53890: [LTO] Record whether LTOUnit splitting is enabled in index

[Peter Collingbourne via Phabricator via llvm-commits]

pcc added a comment.

This means that

  clang -c -flto=thin foo.c
  [upgrade clang]
  clang -c -flto=thin bar.c
  clang foo.o bar.o

will fail unless you add `-fsplit-lto-unit` to the second invocation. Given that the alternative is that we potentially silently miscompile, maybe that's fine. It would be possible to handle this case without aborting or miscompiling by splitting `bar.o` inside the linker and treating it as if it had always been split, but given that there's a workaround maybe that's not that critical.



================
Comment at: lib/Bitcode/Reader/BitcodeReader.cpp:5934
+      // If no flags record found, conservatively return false.
+      return false;
+    case BitstreamEntry::Record:
----------------
If we weren't emitting `FS_FLAGS` into module summaries before then can't we return true here and avoid needing to invert the flag in the bitcode?


================
Comment at: tools/opt/opt.cpp:106
 
+// Enable LTO unit splitting by default for consistency with behavior
+// before adding this option.
----------------
Wouldn't it be better to fix the test cases to be explicit instead?


Repository:
  rL LLVM

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

https://reviews.llvm.org/D53890