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

[Teresa Johnson via llvm-commits]

tejohnson added a comment.

The new comments help a lot. A few minor suggestions. Will add callback to gold-plugin and retry when the coredump resolved.


================
Comment at: include/llvm/LTO/Caching.h:50
@@ +49,3 @@
+///     if (Output.tryLoadFromCache())
+///        return; // Cache hit
+///   }
----------------
Note that when we return and Output is destroyed, the existing entry pulled from cache and passed back to client via CallBack.

================
Comment at: include/llvm/LTO/Caching.h:56
@@ +55,3 @@
+///   OS << ...;
+/// }
+///
----------------
Note that here, when Output is destroyed, the cache entry is updated in the cache and CallBack invoked to pass the contents back to the client.

================
Comment at: include/llvm/LTO/Caching.h:71
@@ +70,3 @@
+public:
+  /// The destructor commit the entry into the cache.
+  ~CacheObjectOutput();
----------------
or pulls it out of the cache if it was already there, and calls AddBuffer.

================
Comment at: include/llvm/LTO/Caching.h:90
@@ +89,3 @@
+  /// Returns true to signal that this implementation of NativeObjectFile
+  /// support caching.!CacheDirectoryPath.empty()
+  bool isCachingEnabled() const override { return true; }
----------------
Remove "!CacheDirectoryPath.empty()"

================
Comment at: lib/LTO/Caching.cpp:53
@@ +52,3 @@
+        // at
+        // the same time.
+        OS << (*ReloadedBufferOrErr)->getBuffer();
----------------
Combine the above 2 comment lines

================
Comment at: lib/LTO/LTO.cpp:88
@@ +87,3 @@
+  }
+
+  // Include the hash for the linkage type to reflect internalization and weak
----------------
Ok got it.


https://reviews.llvm.org/D23599