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

[Teresa Johnson via Phabricator via llvm-commits]

tejohnson added a comment.

In https://reviews.llvm.org/D37993#921444, @kongyi wrote:

> In https://reviews.llvm.org/D37993#893466, @pcc wrote:
>
> > Is it a good idea to support cache pruning in the gold plugin? As I mentioned in https://reviews.llvm.org/D31063 there are some unavoidable race conditions due to limitations in the plugin API.
>
>
> Gold plugin
>
> In https://reviews.llvm.org/D37993#920128, @tejohnson wrote:
>
> > In https://reviews.llvm.org/D37993#918921, @pcc wrote:
> >
> > > In https://reviews.llvm.org/D37993#918918, @tejohnson wrote:
> > >
> > > > In https://reviews.llvm.org/D37993#912428, @pcc wrote:
> > > >
> > > > > In https://reviews.llvm.org/D37993#893466, @pcc wrote:
> > > > >
> > > > > > Is it a good idea to support cache pruning in the gold plugin? As I mentioned in https://reviews.llvm.org/D31063 there are some unavoidable race conditions due to limitations in the plugin API.
> > > > >
> > > > >
> > > > > Ping... not sure whether it's a good idea to ship a known racy feature.
> > > > >
> > > > > If you really want this feature in gold I'd suggest extending the gold plugin API so that it accepts file descriptors, and then have the plugin support this feature conditionally on whether the gold version is new enough.
> > > >
> > > >
> > > > Hi Peter, thanks for pointing this out, and sorry for the delay in responding. Wondering whether instead of removing cache pruning support from gold-plugin for now, we could do something else: write the MemoryBuffer to another temp file and use that path (setting IsTemporary to true so it gets cleaned up). This would obviously require more space on disk, but maybe worth it to enable the pruning safely.
> > >
> > >
> > > Copying to a temporary file sounds fine to me. And if gold ever gets support for file descriptors we could always use the copying code path as a fallback for older versions.
> >
> >
> > Yi, can you address these suggestions?
>
>
> Sure, will fix these in a follow up change.


Ping - Yi, can you fix these?


Repository:
  rL LLVM

https://reviews.llvm.org/D37993