TableGen: Convert to new ActionCache APIs

A few notes based on using ActionCache somewhere:

- If ActionDescription owned its parameters it'd be easier to build.
  Seems a bit error-prone right now.
    - Needed to find a place to store the command-line for "extra data".
      If we wanted more stuff in ExtraData, this'd get pretty tough.
    - Needed to create a SmallVector to hold the InputIDs.
- But, it's a nice property that ActionDescription is immutable.
    - If the content (not just the cache) is mutable then it's hard to
      reason about.
- Potential fix:
    - Make ActionDescription own what it references.
        - Could allow the errors to be lazy about printing the action as
          well (store a copy of the description, and just print in
          `ErrorInfo::log()`).
    - Add ActionDescriptionBuilder, with simple mutating APIs, and a
      constructor `ActionDescription(ActionDescriptionBuilder&&)`.
    - ActionDescriptionRef is lightweight and can be used without
      building (... but maybe this one isn't useful in practice?)
- Conversions between CASID and UniqueIDRef are annoying... but that's
  just a transition problem.

Author: dexonsmith

llvm/include/llvm/CAS/CASDB.h

@@ -329,6 +329,18 @@ class CASDB {
 
   Error getPrintedCASID(CASID ID, SmallVectorImpl<char> &Reference);
 
+  /// Translate from CASID and UniqueIDRef.
+  UniqueIDRef getUniqueID(CASID ID) const {
+    return UniqueIDRef(getNamespace(), ID.getHash());
+  }
+  /// Translate from UniqueIDRef to CASID.
+  CASID getCASID(UniqueIDRef ID) const {
+    assert(&ID.getNamespace() == &getNamespace());
+    return CASID(ID.getHash());
+  }
+  /// Beware of lifetime issues.
+  CASID getCASID(const UniqueID &ID) const = delete;
+
   virtual Expected<BlobRef> createBlob(StringRef Data) = 0;
 
   virtual Expected<TreeRef>

llvm/include/llvm/TableGen/ScanDependencies.h

@@ -19,6 +19,7 @@
 namespace llvm {
 
 namespace cas {
+class ActionCache;
 class CachingOnDiskFileSystem;
 } // end namespace cas
 
@@ -42,8 +43,8 @@ void scanTextForIncludes(StringRef Input, SmallVectorImpl<StringRef> &Includes);
 ///
 /// \p ExecID is the Blob for the running executable. It can be used as part of
 /// the key for caching to avoid (fixed) bugs poisoning results.
-Error scanTextForIncludes(cas::CASDB &CAS, cas::CASID ExecID,
-                          const cas::BlobRef &Blob,
+Error scanTextForIncludes(cas::CASDB &CAS, cas::ActionCache &Cache,
+                          cas::CASID ExecID, const cas::BlobRef &Blob,
                           SmallVectorImpl<StringRef> &Includes);
 
 /// Match logic from SourceMgr::AddIncludeFile.
@@ -57,7 +58,8 @@ Optional<cas::CASID> lookupIncludeID(cas::CachingOnDiskFileSystem &FS,
 ///
 /// \p ExecID is the Blob for the running executable. It can be used as part of
 /// the key for caching to avoid (fixed) bugs poisoning results.
-Error accessAllIncludes(cas::CachingOnDiskFileSystem &FS, cas::CASID ExecID,
+Error accessAllIncludes(cas::ActionCache &Cache,
+                        cas::CachingOnDiskFileSystem &FS, cas::CASID ExecID,
                         ArrayRef<std::string> IncludeDirs,
                         const cas::BlobRef &MainFileBlob);
 
@@ -79,8 +81,8 @@ struct ScanIncludesResult {
 /// it'd be useful for build systems that want to collect dependencies ahead of
 /// time. Maybe can separate a tool/etc. for that.
 Expected<ScanIncludesResult>
-scanIncludes(cas::CASDB &CAS, cas::CASID ExecID, StringRef MainFilename,
-             ArrayRef<std::string> IncludeDirs,
+scanIncludes(cas::CASDB &CAS, cas::ActionCache &Cache, cas::CASID ExecID,
+             StringRef MainFilename, ArrayRef<std::string> IncludeDirs,
              ArrayRef<MappedPrefix> PrefixMappings = None,
              Optional<TreePathPrefixMapper> *CapturedPM = nullptr);
 
@@ -91,8 +93,8 @@ scanIncludes(cas::CASDB &CAS, cas::CASID ExecID, StringRef MainFilename,
 /// \p ExecID is the Blob for the running executable. It can be used as part of
 /// the key for caching to avoid (fixed) bugs poisoning results.
 Expected<ScanIncludesResult>
-scanIncludesAndRemap(cas::CASDB &CAS, cas::CASID ExecID,
-                     std::string &MainFilename,
+scanIncludesAndRemap(cas::CASDB &CAS, cas::ActionCache &Cache,
+                     cas::CASID ExecID, std::string &MainFilename,
                      std::vector<std::string> &IncludeDirs,
                      ArrayRef<MappedPrefix> PrefixMappings);
 

llvm/lib/TableGen/Main.cpp

@@ -19,6 +19,7 @@
 #include "llvm/ADT/ScopeExit.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/StringExtras.h"
+#include "llvm/CAS/ActionCache.h"
 #include "llvm/CAS/CASFileSystem.h"
 #include "llvm/CAS/CachingOnDiskFileSystem.h"
 #include "llvm/CAS/HierarchicalTreeBuilder.h"
@@ -248,10 +249,15 @@ int llvm::TableGenMain(const char *argv0, TableGenMainFn *MainFn) {
 namespace {
 struct TableGenCache {
   std::unique_ptr<cas::CASDB> CAS;
+  std::unique_ptr<cas::ActionCache> Cache;
   Optional<cas::CASID> ExecutableID;
   Optional<cas::CASID> IncludesTreeID;
-  Optional<cas::CASID> ActionID;
-  Optional<cas::CASID> ResultID;
+
+  SmallVector<cas::UniqueIDRef> ActionInputs;
+  SmallString<256> SerializedCommandLine;
+  Optional<cas::ActionDescription> Action;
+
+  Optional<cas::UniqueIDRef> ResultID;
   Optional<cas::CASID> MainFileID;
   std::unique_ptr<MemoryBuffer> MainFile;
   Optional<std::string> OriginalInputFilename;
@@ -264,7 +270,7 @@ struct TableGenCache {
   ~TableGenCache() { CreateDependencyFilePM = nullptr; }
 
   Expected<cas::CASID> createExecutableBlob(StringRef Argv0);
-  Expected<cas::CASID> createCommandLineBlob(ArrayRef<const char *> Args);
+  void serializeCommandLine(ArrayRef<const char *> Args);
   Error createAction(ArrayRef<const char *> Args);
 
   Error lookupCachedResult(ArrayRef<const char *> Args);
@@ -295,15 +301,12 @@ Expected<cas::CASID> TableGenCache::createExecutableBlob(StringRef Argv0) {
   return *Blob;
 }
 
-Expected<cas::CASID>
-TableGenCache::createCommandLineBlob(ArrayRef<const char *> Args) {
-  SmallString<1024> CommandLine;
-
-  // Use raw_svector_stream since it doesn't buffer.
-  raw_svector_ostream OS(CommandLine);
+void TableGenCache::serializeCommandLine(ArrayRef<const char *> Args) {
+  assert(SerializedCommandLine.empty());
+  raw_svector_ostream OS(SerializedCommandLine);
   auto serializeArg = [&](StringRef Arg) {
     OS << Arg;
-    CommandLine.push_back(0);
+    SerializedCommandLine.push_back(0);
   };
   serializeArg(sys::path::filename(Args[0]));
   Args = Args.drop_front();
@@ -340,41 +343,37 @@ TableGenCache::createCommandLineBlob(ArrayRef<const char *> Args) {
     serializeArg(Args.front());
     Args = Args.drop_front();
   }
-
-  Expected<cas::BlobRef> Blob = CAS->createBlob(CommandLine);
-  if (!Blob)
-    return Blob.takeError();
-  return *Blob;
 }
 
 Error TableGenCache::createAction(ArrayRef<const char *> Args) {
-  Expected<cas::CASID> CommandLineID = createCommandLineBlob(Args);
-  if (!CommandLineID)
-    return CommandLineID.takeError();
-
-  cas::HierarchicalTreeBuilder Builder;
-  Builder.push(*IncludesTreeID, cas::TreeEntry::Tree, "includes");
-  Builder.push(*MainFileID, cas::TreeEntry::Regular, "input");
-  Builder.push(*ExecutableID, cas::TreeEntry::Regular, "executable");
-  Builder.push(*CommandLineID, cas::TreeEntry::Regular, "command-line");
-  Expected<cas::TreeRef> Tree = Builder.create(*CAS);
-  if (!Tree)
-    return Tree.takeError();
-  ActionID = *Tree;
+  assert(!Action);
+  serializeCommandLine(Args);
+
+  assert(ActionInputs.empty());
+  ActionInputs.push_back(CAS->getUniqueID(*IncludesTreeID));
+  ActionInputs.push_back(CAS->getUniqueID(*MainFileID));
+  ActionInputs.push_back(CAS->getUniqueID(*ExecutableID));
+  Action.emplace("llvm::tablegen::generate", ActionInputs,
+                 SerializedCommandLine);
   return Error::success();
 }
 
 Error TableGenCache::lookupCachedResult(ArrayRef<const char *> Args) {
   if (Error E =
           cas::createOnDiskCAS(cas::getDefaultOnDiskCASPath()).moveInto(CAS))
     return E;
+  if (Error E = cas::createOnDiskActionCache(
+                    CAS->getNamespace(), cas::getDefaultOnDiskActionCachePath())
+                    .moveInto(Cache))
+    return E;
 
   if (Error E = createExecutableBlob(Args[0]).moveInto(ExecutableID))
     return E;
 
   OriginalInputFilename = InputFilename.getValue();
-  Expected<ScanIncludesResult> Scan = scanIncludesAndRemap(
-      *CAS, *ExecutableID, InputFilename, *&IncludeDirs, PrefixMappings);
+  Expected<ScanIncludesResult> Scan =
+      scanIncludesAndRemap(*CAS, *Cache, *ExecutableID, InputFilename,
+                           *&IncludeDirs, PrefixMappings);
   if (!Scan)
     return Scan.takeError();
 
@@ -387,9 +386,7 @@ Error TableGenCache::lookupCachedResult(ArrayRef<const char *> Args) {
   if (Error E = createAction(Args))
     return E;
 
-  // Not an error for the result to be missing.
-  ResultID = expectedToOptional(CAS->getCachedResult(*ActionID));
-  return Error::success();
+  return Cache->get(*Action, ResultID);
 }
 
 namespace {
@@ -427,7 +424,7 @@ Error TableGenCache::computeResult(TableGenMainFn *MainFn) {
   SmallString<256> DependStorage;
 
   CapturedDiagnostics Diags;
-  if (ActionID) {
+  if (Action) {
     assert(IncludesTreeID && "Expected an input tree...");
     if (auto FS = cas::createCASFileSystem(*CAS, *IncludesTreeID))
       SrcMgr.setFileSystem(std::move(*FS));
@@ -448,7 +445,7 @@ Error TableGenCache::computeResult(TableGenMainFn *MainFn) {
     return E;
 
   // Caching not turned on.
-  if (!ActionID)
+  if (!Action)
     return Error::success();
 
   cas::HierarchicalTreeBuilder Builder;
@@ -472,13 +469,13 @@ Error TableGenCache::computeResult(TableGenMainFn *MainFn) {
   Expected<cas::TreeRef> Tree = Builder.create(*CAS);
   if (!Tree)
     return Tree.takeError();
-  return CAS->putCachedResult(*ActionID, *Tree);
+  return Cache->put(*Action, CAS->getUniqueID(*Tree));
 }
 
 Error TableGenCache::replayResult() {
   assert(ResultID && "Need a result!");
 
-  Expected<cas::TreeRef> Tree = CAS->getTree(*ResultID);
+  Expected<cas::TreeRef> Tree = CAS->getTree(CAS->getCASID(*ResultID));
   if (!Tree)
     return Tree.takeError();
 

llvm/lib/TableGen/ScanDependencies.cpp

@@ -7,6 +7,7 @@
 //===----------------------------------------------------------------------===//
 
 #include "llvm/TableGen/ScanDependencies.h"
+#include "llvm/CAS/ActionCache.h"
 #include "llvm/CAS/CachingOnDiskFileSystem.h"
 #include "llvm/Support/MemoryBuffer.h"
 #include "llvm/Support/PrefixMapper.h"
@@ -83,7 +84,8 @@ fetchCachedIncludedFiles(cas::CASDB &CAS, cas::CASID ID,
   return Error::success();
 }
 
-static Error computeIncludedFiles(cas::CASDB &CAS, cas::CASID Key,
+static Error computeIncludedFiles(cas::CASDB &CAS, cas::ActionCache &Cache,
+                                  const cas::ActionDescription &Action,
                                   StringRef Input,
                                   SmallVectorImpl<StringRef> &IncludedFiles) {
   SmallString<256> ResultToCache;
@@ -93,24 +95,20 @@ static Error computeIncludedFiles(cas::CASDB &CAS, cas::CASID Key,
   Expected<cas::BlobRef> ExpectedResult = CAS.createBlob(ResultToCache);
   if (!ExpectedResult)
     return ExpectedResult.takeError();
-  if (Error E = CAS.putCachedResult(Key, *ExpectedResult))
-    return E;
-  return Error::success();
+  return Cache.put(Action, CAS.getUniqueID(*ExpectedResult));
 }
 
-Error tablegen::scanTextForIncludes(cas::CASDB &CAS, cas::CASID ExecID,
-                                    const cas::BlobRef &Blob,
+Error tablegen::scanTextForIncludes(cas::CASDB &CAS, cas::ActionCache &Cache,
+                                    cas::CASID ExecID, const cas::BlobRef &Blob,
                                     SmallVectorImpl<StringRef> &Includes) {
-  constexpr StringLiteral CacheKeyData = "llvm::tablegen::scanTextForIncludes";
-
-  Expected<cas::NodeRef> Key = CAS.createNode({ExecID, Blob}, CacheKeyData);
-  if (!Key)
-    return Key.takeError();
-
-  if (Optional<cas::CASID> ResultID =
-          expectedToOptional(CAS.getCachedResult(*Key)))
-    return fetchCachedIncludedFiles(CAS, *ResultID, Includes);
-  return computeIncludedFiles(CAS, *Key, Blob.getData(), Includes);
+  cas::UniqueIDRef Inputs[] = {CAS.getUniqueID(ExecID), CAS.getUniqueID(Blob)};
+  cas::ActionDescription Action("llvm::tablegen::scanTextForIncludes", Inputs);
+  Optional<cas::UniqueIDRef> ResultID;
+  if (Error E = Cache.get(Action, ResultID))
+    return E;
+  if (ResultID)
+    return fetchCachedIncludedFiles(CAS, CAS.getCASID(*ResultID), Includes);
+  return computeIncludedFiles(CAS, Cache, Action, Blob.getData(), Includes);
 }
 
 Optional<cas::CASID>
@@ -132,7 +130,8 @@ tablegen::lookupIncludeID(cas::CachingOnDiskFileSystem &FS,
   return ID;
 }
 
-Error tablegen::accessAllIncludes(cas::CachingOnDiskFileSystem &FS,
+Error tablegen::accessAllIncludes(cas::ActionCache &Cache,
+                                  cas::CachingOnDiskFileSystem &FS,
                                   cas::CASID ExecID,
                                   ArrayRef<std::string> IncludeDirs,
                                   const cas::BlobRef &MainFileBlob) {
@@ -157,8 +156,8 @@ Error tablegen::accessAllIncludes(cas::CachingOnDiskFileSystem &FS,
   SmallVector<StringRef> IncludedFiles;
   while (!Worklist.empty()) {
     IncludedFiles.clear();
-    if (Error E = scanTextForIncludes(CAS, ExecID, Worklist.pop_back_val(),
-                                      IncludedFiles))
+    if (Error E = scanTextForIncludes(CAS, Cache, ExecID,
+                                      Worklist.pop_back_val(), IncludedFiles))
       return E;
 
     // Add included files to the worklist. Ignore files not found, since the
@@ -199,10 +198,12 @@ Error tablegen::createMainFileError(StringRef MainFilename,
                                    "': " + EC.message());
 }
 
-Expected<ScanIncludesResult> tablegen::scanIncludes(
-    cas::CASDB &CAS, cas::CASID ExecID, StringRef MainFilename,
-    ArrayRef<std::string> IncludeDirs, ArrayRef<MappedPrefix> PrefixMappings,
-    Optional<TreePathPrefixMapper> *CapturedPM) {
+Expected<ScanIncludesResult>
+tablegen::scanIncludes(cas::CASDB &CAS, cas::ActionCache &Cache,
+                       cas::CASID ExecID, StringRef MainFilename,
+                       ArrayRef<std::string> IncludeDirs,
+                       ArrayRef<MappedPrefix> PrefixMappings,
+                       Optional<TreePathPrefixMapper> *CapturedPM) {
   IntrusiveRefCntPtr<cas::CachingOnDiskFileSystem> FS;
   if (Error E = cas::createCachingOnDiskFileSystem(CAS).moveInto(FS))
     return std::move(E);
@@ -213,7 +214,7 @@ Expected<ScanIncludesResult> tablegen::scanIncludes(
     return MainBlob.takeError();
 
   // Helper for adding to the worklist.
-  if (Error E = accessAllIncludes(*FS, ExecID, IncludeDirs, *MainBlob))
+  if (Error E = accessAllIncludes(Cache, *FS, ExecID, IncludeDirs, *MainBlob))
     return std::move(E);
 
   Optional<TreePathPrefixMapper> LocalPM;
@@ -236,13 +237,13 @@ Expected<ScanIncludesResult> tablegen::scanIncludes(
 }
 
 Expected<ScanIncludesResult>
-tablegen::scanIncludesAndRemap(cas::CASDB &CAS, cas::CASID ExecID,
-                               std::string &MainFilename,
+tablegen::scanIncludesAndRemap(cas::CASDB &CAS, cas::ActionCache &Cache,
+                               cas::CASID ExecID, std::string &MainFilename,
                                std::vector<std::string> &IncludeDirs,
                                ArrayRef<MappedPrefix> PrefixMappings) {
   Optional<TreePathPrefixMapper> PM;
-  auto Result =
-      scanIncludes(CAS, ExecID, MainFilename, IncludeDirs, PrefixMappings, &PM);
+  auto Result = scanIncludes(CAS, Cache, ExecID, MainFilename, IncludeDirs,
+                             PrefixMappings, &PM);
   if (!Result)
     return Result.takeError();