[PATCH] D41279: [ThinLTO][C-API] Correct api comments

Katya Romanova via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Dec 15 15:42:52 PST 2017


kromanova added inline comments.


================
Comment at: include/llvm-c/lto.h:786
+ * Sets the cache pruning interval (in seconds). A negative value sets the
+ * maximum possible pruning interval. An unspecified default value will be
+ * applied, and a value of 0 will be ignored.
----------------
kromanova wrote:
> It looks like we are setting the default values in CachePruning.h, so the comment "An unspecified default value" could be substituted with "Default value of 1200 seconds (20 minutes) is applied".
> 
> Obsolete comments are confusing. When I called this API in Sony's proprietary linker, I relied on this particular comment and end up doing initialization myself in my code. BTW, Ben, since I noticed it, we need to remove this initialization code I talked about from our (Sony's) proprietary linker.  
I would prefer mentioning one more time that negative values effectively disable pruning. Hopefully, after your change the umbrella comment above mentions it, but it will be nice to repeat it the comment directly related to this function.


================
Comment at: include/llvm-c/lto.h:787
+ * maximum possible pruning interval. An unspecified default value will be
+ * applied, and a value of 0 will be ignored.
  *
----------------
Also, a value of 0 is not ignored, but instead it forces the scan to occur. See comment in CachePruning.h 

However, please do not make this change now, leave it "as is". I will send a patch a little later to accept 0 interval in thinlto_codegen_set_cache_pruning_function (as well as in 2 other APIs for setting relative size and expiration).  I will do this change in the comment(s) at the same time as I do the functional change itself. 



https://reviews.llvm.org/D41279





More information about the llvm-commits mailing list