[PATCH] D37993: [ThinLTO/gold] Implement ThinLTO cache pruning support

[Teresa Johnson via Phabricator via llvm-commits]

tejohnson added inline comments.


================
Comment at: llvm/trunk/tools/gold/gold-plugin.cpp:980
+  // Prune cache
+  if (!options::cache_policy.empty()) {
+    CachePruningPolicy policy = check(parseCachePruningPolicy(options::cache_policy));
----------------
pcc wrote:
> tejohnson wrote:
> > pcc wrote:
> > > tejohnson wrote:
> > > > pcc wrote:
> > > > > I think this should check `cache_dir`, not `cache_policy`.
> > > > It seems fine since pruneCache will return immediately if cache_dir is empty. But we could save some work if both were checked. I assume parseCachePruningPolicy will also work if cache_policy is empty, but it seems fine to have the checking.
> > > I was more thinking about the behaviour when a cache directory is specified and no cache policy is specified. In that case the behaviour of the other linkers is to prune with the default policy, so it's probably best to be consistent with that.
> > Oh, I see what you are saying. Yes, this makes sense.
> Please at least address this.
Since we haven't heard from Yi, I will go ahead and fix these. Just mailed you D43389 to fix this issue, I'll address the possible race separately.


Repository:
  rL LLVM

https://reviews.llvm.org/D37993