[PATCH] D24622: LTO: Simplify caching interface.

Mehdi AMINI via llvm-commits llvm-commits at lists.llvm.org
Thu Sep 22 22:25:21 PDT 2016


mehdi_amini added inline comments.

================
Comment at: include/llvm/LTO/Config.h:60
@@ +59,3 @@
+/// cache, pass a unique string as the Key. For hits, the cached file will be
+/// added using AddFile and this function will return AddStreamFn(). For misses,
+/// the cache will return a stream callback which must be called at most once to
----------------
The "AddFile" part seems convoluted to me. It seems to be here only to satisfy the flow of having a stream as a returned type.

================
Comment at: include/llvm/LTO/Config.h:72
@@ -64,2 +71,3 @@
+    NativeObjectCache;
 
 /// LTO configuration. A linker can configure LTO by setting fields in this data
----------------
Since you're claiming to make it easier to change the cache implementation in the description, can you clarify how would I implement a "in-memory" cache with this interface?

================
Comment at: include/llvm/LTO/Config.h:116
@@ -107,1 +115,3 @@
 
+  NativeObjectCache Cache;
+
----------------
No documented.

================
Comment at: include/llvm/LTO/LTO.h:316
@@ +315,3 @@
+  /// AddFile functions to add native object files to the link.
+  ///
+  /// The client will receive at most one callback (via either AddStream or
----------------
> Is there any specific client you have in mind where this would be a problem?

That's not the angle I'm looking at when looking at the API design.

For instance:
Why is `NativeObjectCache Cache` in in the config? While these callbacks are here? 



https://reviews.llvm.org/D24622





More information about the llvm-commits mailing list