[PATCH] D23599: [ThinLTO] Add caching to the new LTO API

[Teresa Johnson via llvm-commits]

tejohnson added inline comments.

================
Comment at: lib/LTO/Caching.cpp:71
@@ +70,3 @@
+  assert(!EntryPath.empty() && "API Violation: client didn't call "
+                               "tryLoadFromCache() before getStream()");
+  // Write to a temporary to avoid race condition
----------------
My gold testing is hitting this assert many times. The reason is that when we call back to AddOutput, we don't know whether this is a regular or thin LTO call, so if there is a CacheDir specified we always create a CacheObjectOutput. If it is regularLTO, then we will eventually call getStream() from codegen without previously calling tryLoadFromCache.

This is happening in a couple of instances:
- since for ThinLTO when we have save-temps we setup a hook and are still unconditionally creating a combined LTO module and trying to code gen it
- my initial round of gold-plugin support was being lazy just for testing and always setting a CacheDir to something hardcoded (this would obviously go away and be set via a plugin-opt)

Even with the above two scenarios fixed, we want to support a mixed Thin and Regular LTO compilation model, so it needs to be handled. The easiest way seems to be to add a parameter to the AddOutput callback that takes an indication of which type of LTO this file is.


https://reviews.llvm.org/D23599