[PATCH] D24622: LTO: Simplify caching interface.

Peter Collingbourne via llvm-commits llvm-commits at lists.llvm.org
Thu Sep 22 14:07:59 PDT 2016


pcc added inline comments.

================
Comment at: tools/gold/gold-plugin.cpp:801
@@ -800,3 +785,1 @@
                       MaxTasks > 1 ? Task : -1);
-    IsTemporary[Task] = !SaveTemps && options::cache_dir.empty();
-    if (options::cache_dir.empty())
----------------
tejohnson wrote:
> pcc wrote:
> > tejohnson wrote:
> > > pcc wrote:
> > > > tejohnson wrote:
> > > > > Removing the "&& options::cache_dir.empty()" seems like it would cause gold-plugin to remove the cache file, which is why I added that here when I added caching.
> > > > The only files created by AddStream are temporary (modulo save-temps), which means that they cannot be cache files.
> > > > 
> > > > (On a related note I just noticed that this patch also fixes a bug in the old code: if an output was cached gold would create an unused temporary file.)
> > > In r279883 I changed this so that the path to the cache entry is passed back to gold instead of the temp file (see where we set OutputName = EntryPath below. If IsTemporary is not set to false, then later when we call recordFile we end up adding Filename to the Cleanup vector which means it is removed later in the cleanup_hook.
> > > 
> > > But yes, it looks like we should not be calling getOutputFileName in the caching case.
> > The code where we set OutputName = EntryPath has been moved to AddFile. This callback isn't called at all if the file is cached (i.e. if we call AddFile for this output).
> But it is called on a cache miss, where you also call AddFile and will use the cache entry directly. Looks like that happens when you destroy CacheStream. That's what I believe we are doing currently. So we don't want to remove the Filenames[Task] in that case either.
In the case of a cache miss we use an AddStream provided by the cache, which does not call the AddStream provided by the client.


https://reviews.llvm.org/D24622





More information about the llvm-commits mailing list