[PATCH] D111371: [Support][ThinLTO] Move ThinLTO caching to LLVM Support library.

Fri Oct 8 11:30:27 PDT 2021

noajshu marked 3 inline comments as done.
noajshu added inline comments.

================
Comment at: llvm/lib/Support/Caching.cpp:36

+  SmallString<10> CacheName = CacheNameRef;
   return [=](unsigned Task, StringRef Key) -> AddStreamFn {
----------------
tejohnson wrote:
> Is this copy necessary? I believe sys::path::append takes a Twine and eventually makes a copy of it.
Thanks, removed!

================
Comment at: llvm/lib/Support/Caching.cpp:32
+                                             AddBufferFn AddBuffer,
+                                             StringRef CacheNameRef) {
   if (std::error_code EC = sys::fs::create_directories(CacheDirectoryPath))
----------------
phosek wrote:
> Can we make this a `Twine`? I'd also make this argument the first one.
Done! Also split out the customized cache name and file prefixes, to avoid changing any behavior. In ThinLTO these were "ThinLTO" and "Thin" respectively.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D111371/new/

https://reviews.llvm.org/D111371