[PATCH] D42267: [ThinLTO] Allow 0 to be a valid value for pruning interval for C LTO API.

Steven Wu via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Feb 6 16:22:44 PST 2018


steven_wu added a comment.

In https://reviews.llvm.org/D42267#999761, @kromanova wrote:

> > I am definitely not against the change. It would be good to have the number means the same thing in C/C++ API. I think ld64 can be updated in advance before we release 7.0 so we can have a larger compatibility window so I don't think this should be a blocker.
>
> So, can I assume that I got an "OK" from the other main stakeholder who is using C LTO API? BTW, I CC-ed you to another small patch https://reviews.llvm.org/D42446, where I added a couple of missing APIs for controlling cache policy. You might want to expose it to ld64 users.


Adding new APIs to C APIs is safe for us. Thanks for the heads up.

> 
> 
>> I don't really like that we have the default in multiple places depending on which tool you use. So it seems we have default pruning interval in CachePruningPolicy and llvm-lto (and maybe in some linker that uses the C/C++ interfaces as well). That is probably the reason why C API choose to ignore 0 because if you have a cl::opt initialize to zero, it means it will respect the default from CachePruningPolicy.
> 
> I agree. Do you have an idea how to fix this? Feel free to propose a patch.

Unless you want to reserve a number for that (-2?) which is quite confusing, I don't know how to improve this situation. This patch is LGTM now.


Repository:
  rL LLVM

https://reviews.llvm.org/D42267





More information about the llvm-commits mailing list