gamesh411 updated this revision to Diff 211361.
gamesh411 added a comment.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Refactor functionality into local classes


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64753

Files:
  clang/include/clang/CrossTU/CrossTranslationUnit.h
  clang/lib/CrossTU/CrossTranslationUnit.cpp
  llvm/test/CodeGen/X86/packss.ll

Index: llvm/test/CodeGen/X86/packss.ll
===================================================================
--- llvm/test/CodeGen/X86/packss.ll
+++ llvm/test/CodeGen/X86/packss.ll
@@ -266,7 +266,7 @@
 }
 
 define <16 x i8> @packsswb_icmp_128_zero(<8 x i16> %a0) {
-; SSE-LABEL: packsswb_128_zero:
+; SSE-LABEL: packsswb_icmp_128_zero:
 ; SSE:       # %bb.0:
 ; SSE-NEXT:    pxor %xmm1, %xmm1
 ; SSE-NEXT:    pcmpeqw %xmm0, %xmm1
@@ -274,7 +274,7 @@
 ; SSE-NEXT:    movq {{.*#+}} xmm0 = xmm1[0],zero
 ; SSE-NEXT:    ret{{[l|q]}}
 ;
-; AVX-LABEL: packsswb_128_zero:
+; AVX-LABEL: packsswb_icmp_128_zero:
 ; AVX:       # %bb.0:
 ; AVX-NEXT:    vpxor %xmm1, %xmm1, %xmm1
 ; AVX-NEXT:    vpcmpeqw %xmm1, %xmm0, %xmm0
@@ -287,7 +287,7 @@
 }
 
 define <32 x i8> @packsswb_icmp_zero_256(<16 x i16> %a0) {
-; SSE-LABEL: packsswb_zero_256:
+; SSE-LABEL: packsswb_icmp_zero_256:
 ; SSE:       # %bb.0:
 ; SSE-NEXT:    pxor %xmm2, %xmm2
 ; SSE-NEXT:    pcmpeqw %xmm2, %xmm1
@@ -298,7 +298,7 @@
 ; SSE-NEXT:    movaps %xmm2, %xmm1
 ; SSE-NEXT:    ret{{[l|q]}}
 ;
-; AVX1-LABEL: packsswb_zero_256:
+; AVX1-LABEL: packsswb_icmp_zero_256:
 ; AVX1:       # %bb.0:
 ; AVX1-NEXT:    vextractf128 $1, %ymm0, %xmm1
 ; AVX1-NEXT:    vpxor %xmm2, %xmm2, %xmm2
@@ -311,7 +311,7 @@
 ; AVX1-NEXT:    vblendps {{.*#+}} ymm0 = ymm1[0,1],ymm0[2,3],ymm1[4,5],ymm0[6,7]
 ; AVX1-NEXT:    ret{{[l|q]}}
 ;
-; AVX2-LABEL: packsswb_zero_256:
+; AVX2-LABEL: packsswb_icmp_zero_256:
 ; AVX2:       # %bb.0:
 ; AVX2-NEXT:    vpxor %xmm1, %xmm1, %xmm1
 ; AVX2-NEXT:    vpcmpeqw %ymm1, %ymm0, %ymm0
Index: clang/lib/CrossTU/CrossTranslationUnit.cpp
===================================================================
--- clang/lib/CrossTU/CrossTranslationUnit.cpp
+++ clang/lib/CrossTU/CrossTranslationUnit.cpp
@@ -18,8 +18,8 @@
 #include "clang/Frontend/CompilerInstance.h"
 #include "clang/Frontend/TextDiagnosticPrinter.h"
 #include "clang/Index/USRGeneration.h"
-#include "llvm/ADT/Triple.h"
 #include "llvm/ADT/Statistic.h"
+#include "llvm/ADT/Triple.h"
 #include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/ManagedStatic.h"
 #include "llvm/Support/Path.h"
@@ -65,8 +65,7 @@
       Rhs.getVendor() != Triple::UnknownVendor &&
       Lhs.getVendor() != Rhs.getVendor())
     return false;
-  if (!Lhs.isOSUnknown() && !Rhs.isOSUnknown() &&
-      Lhs.getOS() != Rhs.getOS())
+  if (!Lhs.isOSUnknown() && !Rhs.isOSUnknown() && Lhs.getOS() != Rhs.getOS())
     return false;
   if (Lhs.getEnvironment() != Triple::UnknownEnvironment &&
       Rhs.getEnvironment() != Triple::UnknownEnvironment &&
@@ -188,8 +187,8 @@
 }
 
 CrossTranslationUnitContext::CrossTranslationUnitContext(CompilerInstance &CI)
-    : CI(CI), Context(CI.getASTContext()),
-      CTULoadThreshold(CI.getAnalyzerOpts()->CTUImportThreshold) {}
+    : CI(CI), Context(CI.getASTContext()), ASTStorage(CI),
+      CTULoadGuard(CI.getAnalyzerOpts()->CTUImportThreshold) {}
 
 CrossTranslationUnitContext::~CrossTranslationUnitContext() {}
 
@@ -237,8 +236,8 @@
   if (LookupName.empty())
     return llvm::make_error<IndexError>(
         index_error_code::failed_to_generate_usr);
-  llvm::Expected<ASTUnit *> ASTUnitOrError = loadExternalAST(
-      LookupName, CrossTUDir, IndexName, DisplayCTUProgress);
+  llvm::Expected<ASTUnit *> ASTUnitOrError =
+      loadExternalAST(LookupName, CrossTUDir, IndexName, DisplayCTUProgress);
   if (!ASTUnitOrError)
     return ASTUnitOrError.takeError();
   ASTUnit *Unit = *ASTUnitOrError;
@@ -340,6 +339,144 @@
   }
 }
 
+CrossTranslationUnitContext::LoadGuard::LoadGuard(unsigned CTULoadThreshold)
+    : CTULoadThreshold(CTULoadThreshold), NumASTLoaded(0u) {}
+
+CrossTranslationUnitContext::LoadPass
+CrossTranslationUnitContext::LoadGuard::beginLoad() {
+  bool CanBegin = NumASTLoaded < CTULoadThreshold;
+  return {NumASTLoaded, CanBegin};
+}
+
+CrossTranslationUnitContext::LoadPass::LoadPass(unsigned &NumASTLoaded,
+                                                bool CanBegin)
+    : NumASTLoaded(NumASTLoaded), CanBegin(CanBegin), WasSuccessful(false) {}
+
+CrossTranslationUnitContext::LoadPass::~LoadPass() {
+  if (WasSuccessful)
+    ++NumASTLoaded;
+}
+
+void CrossTranslationUnitContext::LoadPass::wasSuccessful() {
+  WasSuccessful = true;
+}
+
+CrossTranslationUnitContext::LoadPass::operator bool() const {
+  return CanBegin;
+}
+
+CrossTranslationUnitContext::ASTDumpLoader::ASTDumpLoader(
+    const CompilerInstance &CI)
+    : CI(CI) {}
+
+std::unique_ptr<ASTUnit>
+CrossTranslationUnitContext::ASTDumpLoader::operator()(StringRef ASTFileName) {
+  // Load AST from ast-dump.
+  IntrusiveRefCntPtr<DiagnosticOptions> DiagOpts = new DiagnosticOptions();
+  TextDiagnosticPrinter *DiagClient =
+      new TextDiagnosticPrinter(llvm::errs(), &*DiagOpts);
+  IntrusiveRefCntPtr<DiagnosticIDs> DiagID(new DiagnosticIDs());
+  IntrusiveRefCntPtr<DiagnosticsEngine> Diags(
+      new DiagnosticsEngine(DiagID, &*DiagOpts, DiagClient));
+
+  return ASTUnit::LoadFromASTFile(
+      ASTFileName, CI.getPCHContainerOperations()->getRawReader(),
+      ASTUnit::LoadEverything, Diags, CI.getFileSystemOpts());
+}
+
+CrossTranslationUnitContext::ASTUnitStorage::ASTUnitStorage(
+    const CompilerInstance &CI)
+    : FileAccessor(CI) {}
+
+llvm::Expected<ASTUnit *>
+CrossTranslationUnitContext::ASTUnitStorage::getASTUnitForFile(StringRef FileName) {
+  // Try the cache first.
+  auto ASTCacheEntry = FileASTUnitMap.find(FileName);
+  if (ASTCacheEntry == FileASTUnitMap.end()) {
+    // Load the ASTUnit from the pre-dumped AST file specified by ASTFileName.
+    std::unique_ptr<ASTUnit> LoadedUnit = FileAccessor(FileName);
+
+    // Need the raw pointer and the unique_ptr as well.
+    ASTUnit* Unit = LoadedUnit.get();
+
+    // Update the cache.
+    FileASTUnitMap[FileName] = std::move(LoadedUnit);
+    return Unit;
+
+  } else {
+    // Found in the cache.
+    return ASTCacheEntry->second.get();
+  }
+}
+
+llvm::Expected<ASTUnit *>
+CrossTranslationUnitContext::ASTUnitStorage::getASTUnitForFunction(
+    StringRef FunctionName, StringRef CrossTUDir, StringRef IndexName) {
+  // Try the cache first.
+  auto ASTCacheEntry = NameASTUnitMap.find(FunctionName);
+  if (ASTCacheEntry == NameASTUnitMap.end()) {
+    // Load the ASTUnit from the pre-dumped AST file specified by ASTFileName.
+
+    // Ensure that the Index is loaded, as we need to search in it.
+    if (llvm::Error IndexLoadError =
+            ensureCTUIndexLoaded(CrossTUDir, IndexName))
+      return std::move(IndexLoadError);
+
+    // Check if there is and entry in the index for the function.
+    if (!NameFileMap.count(FunctionName)) {
+      ++NumNotInOtherTU;
+      return llvm::make_error<IndexError>(index_error_code::missing_definition);
+    }
+
+    // Search in the index for the filename where the definition of FuncitonName
+    // resides.
+    if (llvm::Expected<ASTUnit *> FoundForFile =
+            getASTUnitForFile(NameFileMap[FunctionName])) {
+
+      // Update the cache.
+      NameASTUnitMap[FunctionName] = *FoundForFile;
+      return *FoundForFile;
+
+    } else {
+      return FoundForFile.takeError();
+    }
+  } else {
+    // Found in the cache.
+    return ASTCacheEntry->second;
+  }
+}
+
+llvm::Expected<std::string>
+CrossTranslationUnitContext::ASTUnitStorage::getFileForFunction(
+    StringRef FunctionName, StringRef CrossTUDir, StringRef IndexName) {
+  if (llvm::Error IndexLoadError = ensureCTUIndexLoaded(CrossTUDir, IndexName))
+    return std::move(IndexLoadError);
+  return NameFileMap[FunctionName];
+}
+
+llvm::Error CrossTranslationUnitContext::ASTUnitStorage::ensureCTUIndexLoaded(
+    StringRef CrossTUDir, StringRef IndexName) {
+  // Dont initialize if the map is filled.
+  if (!NameFileMap.empty())
+    return llvm::Error::success();
+
+  // Get the absolute path to the index file.
+  SmallString<256> IndexFile = CrossTUDir;
+  if (llvm::sys::path::is_absolute(IndexName))
+    IndexFile = IndexName;
+  else
+    llvm::sys::path::append(IndexFile, IndexName);
+
+  if (auto IndexMapping = parseCrossTUIndex(IndexFile, CrossTUDir)) {
+    // Initialize member map.
+    NameFileMap = *IndexMapping;
+    return llvm::Error::success();
+  } else {
+    // Error while parsing CrossTU index file.
+    return IndexMapping.takeError();
+  };
+}
+
 llvm::Expected<ASTUnit *> CrossTranslationUnitContext::loadExternalAST(
     StringRef LookupName, StringRef CrossTUDir, StringRef IndexName,
     bool DisplayCTUProgress) {
@@ -348,64 +485,41 @@
   //        translation units contains decls with the same lookup name an
   //        error will be returned.
 
-  if (NumASTLoaded >= CTULoadThreshold) {
+  using LoadPass = CrossTranslationUnitContext::LoadPass;
+
+  // RAII incrementing counter is used to count successful loads.
+  LoadPass LoadAttempt = CTULoadGuard.beginLoad();
+
+  // If import threshold is reached, don't import anything.
+  if (!LoadAttempt) {
     ++NumASTLoadThresholdReached;
     return llvm::make_error<IndexError>(
         index_error_code::load_threshold_reached);
   }
 
-  ASTUnit *Unit = nullptr;
-  auto NameUnitCacheEntry = NameASTUnitMap.find(LookupName);
-  if (NameUnitCacheEntry == NameASTUnitMap.end()) {
-    if (NameFileMap.empty()) {
-      SmallString<256> IndexFile = CrossTUDir;
-      if (llvm::sys::path::is_absolute(IndexName))
-        IndexFile = IndexName;
-      else
-        llvm::sys::path::append(IndexFile, IndexName);
-      llvm::Expected<llvm::StringMap<std::string>> IndexOrErr =
-          parseCrossTUIndex(IndexFile, CrossTUDir);
-      if (IndexOrErr)
-        NameFileMap = *IndexOrErr;
-      else
-        return IndexOrErr.takeError();
-    }
+  // Try to get the value from the heavily cached storage.
+  llvm::Expected<ASTUnit *> Unit =
+      ASTStorage.getASTUnitForFunction(LookupName, CrossTUDir, IndexName);
 
-    auto It = NameFileMap.find(LookupName);
-    if (It == NameFileMap.end()) {
-      ++NumNotInOtherTU;
-      return llvm::make_error<IndexError>(index_error_code::missing_definition);
-    }
-    StringRef ASTFileName = It->second;
-    auto ASTCacheEntry = FileASTUnitMap.find(ASTFileName);
-    if (ASTCacheEntry == FileASTUnitMap.end()) {
-      IntrusiveRefCntPtr<DiagnosticOptions> DiagOpts = new DiagnosticOptions();
-      TextDiagnosticPrinter *DiagClient =
-          new TextDiagnosticPrinter(llvm::errs(), &*DiagOpts);
-      IntrusiveRefCntPtr<DiagnosticIDs> DiagID(new DiagnosticIDs());
-      IntrusiveRefCntPtr<DiagnosticsEngine> Diags(
-          new DiagnosticsEngine(DiagID, &*DiagOpts, DiagClient));
-
-      std::unique_ptr<ASTUnit> LoadedUnit(ASTUnit::LoadFromASTFile(
-          ASTFileName, CI.getPCHContainerOperations()->getRawReader(),
-          ASTUnit::LoadEverything, Diags, CI.getFileSystemOpts()));
-      Unit = LoadedUnit.get();
-      FileASTUnitMap[ASTFileName] = std::move(LoadedUnit);
-      ++NumASTLoaded;
-      if (DisplayCTUProgress) {
-        llvm::errs() << "CTU loaded AST file: "
-                     << ASTFileName << "\n";
-      }
-    } else {
-      Unit = ASTCacheEntry->second.get();
-    }
-    NameASTUnitMap[LookupName] = Unit;
-  } else {
-    Unit = NameUnitCacheEntry->second;
-  }
   if (!Unit)
+    return Unit.takeError();
+
+  // Check whether the backing pointer of the Expected is a nullptr.
+  if (!*Unit)
     return llvm::make_error<IndexError>(
         index_error_code::failed_to_get_external_ast);
+
+  // The backing pointer is not null, loading was successful.
+  LoadAttempt.wasSuccessful();
+
+  if (DisplayCTUProgress) {
+    if (llvm::Expected<std::string> FileName =
+            ASTStorage.getFileForFunction(LookupName, CrossTUDir, IndexName))
+      llvm::errs() << "CTU loaded AST file: " << *FileName << "\n";
+    else
+      return FileName.takeError();
+  }
+
   return Unit;
 }
 
@@ -417,20 +531,19 @@
   ASTImporter &Importer = getOrCreateASTImporter(D->getASTContext());
   auto ToDeclOrError = Importer.Import(D);
   if (!ToDeclOrError) {
-    handleAllErrors(ToDeclOrError.takeError(),
-                    [&](const ImportError &IE) {
-                      switch (IE.Error) {
-                      case ImportError::NameConflict:
-                        ++NumNameConflicts;
-                         break;
-                      case ImportError::UnsupportedConstruct:
-                        ++NumUnsupportedNodeFound;
-                        break;
-                      case ImportError::Unknown:
-                        llvm_unreachable("Unknown import error happened.");
-                        break;
-                      }
-                    });
+    handleAllErrors(ToDeclOrError.takeError(), [&](const ImportError &IE) {
+      switch (IE.Error) {
+      case ImportError::NameConflict:
+        ++NumNameConflicts;
+        break;
+      case ImportError::UnsupportedConstruct:
+        ++NumUnsupportedNodeFound;
+        break;
+      case ImportError::Unknown:
+        llvm_unreachable("Unknown import error happened.");
+        break;
+      }
+    });
     return llvm::make_error<IndexError>(index_error_code::failed_import);
   }
   auto *ToDecl = cast<T>(*ToDeclOrError);
Index: clang/include/clang/CrossTU/CrossTranslationUnit.h
===================================================================
--- clang/include/clang/CrossTU/CrossTranslationUnit.h
+++ clang/include/clang/CrossTU/CrossTranslationUnit.h
@@ -166,28 +166,99 @@
   void lazyInitImporterSharedSt(TranslationUnitDecl *ToTU);
   ASTImporter &getOrCreateASTImporter(ASTContext &From);
   template <typename T>
-  llvm::Expected<const T *> getCrossTUDefinitionImpl(const T *D,
-                                                     StringRef CrossTUDir,
-                                                     StringRef IndexName,
-                                                     bool DisplayCTUProgress);
+  llvm::Expected<const T *>
+  getCrossTUDefinitionImpl(const T *D, StringRef CrossTUDir,
+                           StringRef IndexName, bool DisplayCTUProgress);
   template <typename T>
-  const T *findDefInDeclContext(const DeclContext *DC,
-                                StringRef LookupName);
+  const T *findDefInDeclContext(const DeclContext *DC, StringRef LookupName);
   template <typename T>
   llvm::Expected<const T *> importDefinitionImpl(const T *D);
 
-  llvm::StringMap<std::unique_ptr<clang::ASTUnit>> FileASTUnitMap;
-  llvm::StringMap<clang::ASTUnit *> NameASTUnitMap;
-  llvm::StringMap<std::string> NameFileMap;
   llvm::DenseMap<TranslationUnitDecl *, std::unique_ptr<ASTImporter>>
       ASTUnitImporterMap;
   CompilerInstance &CI;
   ASTContext &Context;
   std::shared_ptr<ASTImporterSharedState> ImporterSharedSt;
-  /// \p CTULoadTreshold should serve as an upper limit to the number of TUs
-  /// imported in order to reduce the memory footprint of CTU analysis.
-  const unsigned CTULoadThreshold;
-  unsigned NumASTLoaded{0u};
+
+  /// Cached access to ASTUnit mapping information is implemented in this
+  /// section.
+
+  /// Functor for loading ASTUnits from AST-dump files.
+  class ASTDumpLoader {
+  public:
+    ASTDumpLoader(const CompilerInstance &CI);
+    std::unique_ptr<ASTUnit> operator()(StringRef ASTFileName);
+
+  private:
+    const CompilerInstance &CI;
+  };
+
+  /// Storage for ASTUnits, cached access, and providing searchability are the
+  /// concerns of ASTUnitStorage class.
+  class ASTUnitStorage {
+  public:
+    ASTUnitStorage(const CompilerInstance &CI);
+    llvm::Expected<ASTUnit *> getASTUnitForFunction(StringRef FunctionName,
+                                                    StringRef CrossTUDir,
+                                                    StringRef IndexName);
+    llvm::Expected<std::string> getFileForFunction(StringRef FunctionName,
+                                                   StringRef CrossTUDir,
+                                                   StringRef IndexName);
+
+  private:
+    llvm::Error ensureCTUIndexLoaded(StringRef CrossTUDir, StringRef IndexName);
+    llvm::Expected<ASTUnit *> getASTUnitForFile(StringRef FileName);
+
+    template <typename... T> using BaseMapTy = llvm::StringMap<T...>;
+    using OwningMapTy = BaseMapTy<std::unique_ptr<clang::ASTUnit>>;
+    using NonOwningMapTy = BaseMapTy<clang::ASTUnit *>;
+
+    OwningMapTy FileASTUnitMap;
+    NonOwningMapTy NameASTUnitMap;
+
+    using IndexMapTy = BaseMapTy<std::string>;
+    IndexMapTy NameFileMap;
+
+    ASTDumpLoader FileAccessor;
+  };
+
+  ASTUnitStorage ASTStorage;
+
+  /// The thresholding of AST-loads is implemented in this section.
+
+  /// RAII counter to signal threshold reached condition, and to increment the
+  /// counter on successful load. Member `CanBegin` is used to signal, that the
+  /// import attempt should be made at the beginning. Member `WasSuccesful`
+  /// signifies whether the load is successfully finished. The counter is
+  /// incremented if the instance is destroyed while `WasSuccessful` is true.
+  class LoadPass {
+  public:
+    LoadPass(unsigned &NumASTLoaded, bool CanBegin);
+    ~LoadPass();
+    void wasSuccessful();
+    operator bool() const;
+
+  private:
+    unsigned &NumASTLoaded;
+    bool CanBegin;
+    bool WasSuccessful;
+  };
+
+  /// An ASTLoadGuard instance manages the threshold and AST loaded values, and
+  /// it is responsible for handing out LoadPass instances.
+  class LoadGuard {
+  public:
+    LoadGuard(unsigned CTULoadThreshold);
+    LoadPass beginLoad();
+
+  private:
+    /// \p CTULoadTreshold should serve as an upper limit to the number of TUs
+    /// imported in order to reduce the memory footprint of CTU analysis.
+    const unsigned CTULoadThreshold;
+    unsigned NumASTLoaded;
+  };
+
+  LoadGuard CTULoadGuard;
 };
 
 } // namespace cross_tu
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to