Handle recursion in LLVMIRGeneration Timer

[davide]

Summary

See http://reviews.llvm.org/D20735 for more context.
Implementing this clang-side is not as terrible as I originally thought. Maybe needs a test case, but I wasn't able to reduce one easily (yet).

[vsk]

Comments on this patch -- The increment and decrement snippets seem like they could be pulled into helper methods. That should make it easier to update all users of LLVMIRGeneration. I wasn't able to come up with a regression test, but I do think this patch needs one.

Overall comments -- My main concern is that this isn't the only affected user of the Timer class. I have a preference for D20735 because it defines the underlying problem away. I'll defer to someone else on this (maybe @rafael?).

[vsk]

Ping, any updates on this patch?

[• rafael]

Since we found only one user, I think my preference is to handle it there.

Cheers,
Rafael

[davide]

ping?

[davide]

Richard, can you please take a look at this? The more I look at it the more it seems weird that we can recurse in this case, but I may miss something

cfe/trunk/lib/CodeGen/CodeGenAction.cpp

class BackendConsumer : public ASTConsumer {
     BackendAction Action;
     const CodeGenOptions &CodeGenOpts;
     const TargetOptions &TargetOpts;
     const LangOptions &LangOpts;
     std::unique_ptr<raw_pwrite_stream> AsmOutStream;
     ASTContext *Context;
 
     Timer LLVMIRGeneration;
+    unsigned LLVMIRGenerationRefCount;
 
     std::unique_ptr<CodeGenerator> Gen;
 
     SmallVector<std::pair<unsigned, std::unique_ptr<llvm::Module>>, 4>
         LinkModules;
 
     // This is here so that the diagnostic printer knows the module a diagnostic
     // refers to.
     llvm::Module *CurLinkModule = nullptr;
 
   public:
     BackendConsumer(
         BackendAction Action, DiagnosticsEngine &Diags,
         const HeaderSearchOptions &HeaderSearchOpts,
         const PreprocessorOptions &PPOpts, const CodeGenOptions &CodeGenOpts,
         const TargetOptions &TargetOpts, const LangOptions &LangOpts,
         bool TimePasses, const std::string &InFile,
         const SmallVectorImpl<std::pair<unsigned, llvm::Module *>> &LinkModules,
         std::unique_ptr<raw_pwrite_stream> OS, LLVMContext &C,
         CoverageSourceInfo *CoverageInfo = nullptr)
         : Diags(Diags), Action(Action), CodeGenOpts(CodeGenOpts),
           TargetOpts(TargetOpts), LangOpts(LangOpts),
           AsmOutStream(std::move(OS)), Context(nullptr),
           LLVMIRGeneration("LLVM IR Generation Time"),
+          LLVMIRGenerationRefCount(0),
           Gen(CreateLLVMCodeGen(Diags, InFile, HeaderSearchOpts, PPOpts,
                                 CodeGenOpts, C, CoverageInfo)) {
       llvm::TimePassesIsEnabled = TimePasses;
       for (auto &I : LinkModules)
         this->LinkModules.push_back(
             std::make_pair(I.first, std::unique_ptr<llvm::Module>(I.second)));
     }
     llvm::Module *getModule() const { return Gen->GetModule(); }
void Initialize(ASTContext &Ctx) override {
         LLVMIRGeneration.stopTimer();
     }
 
     bool HandleTopLevelDecl(DeclGroupRef D) override {
       PrettyStackTraceDecl CrashInfo(*D.begin(), SourceLocation(),
                                      Context->getSourceManager(),
                                      "LLVM IR generation of declaration");
 
-      if (llvm::TimePassesIsEnabled)
+      // Recurse.
+      if (llvm::TimePassesIsEnabled) {
+        LLVMIRGenerationRefCount += 1;
+        if (LLVMIRGenerationRefCount == 1)
           LLVMIRGeneration.startTimer();
+      }
 
       Gen->HandleTopLevelDecl(D);
 
-      if (llvm::TimePassesIsEnabled)
+      if (llvm::TimePassesIsEnabled) {
+        LLVMIRGenerationRefCount -= 1;
+        if (LLVMIRGenerationRefCount == 0)
           LLVMIRGeneration.stopTimer();
+      }
 
       return true;
     }
 
     void HandleInlineFunctionDefinition(FunctionDecl *D) override {
       PrettyStackTraceDecl CrashInfo(D, SourceLocation(),
                                      Context->getSourceManager(),
                                      "LLVM IR generation of inline function");
       if (llvm::TimePassesIsEnabled)
         LLVMIRGeneration.startTimer();
 
       Gen->HandleInlineFunctionDefinition(D);
 
       if (llvm::TimePassesIsEnabled)
         LLVMIRGeneration.stopTimer();
     }
 
     void HandleTranslationUnit(ASTContext &C) override {
       {
         PrettyStackTraceString CrashInfo("Per-file LLVM IR generation");
-        if (llvm::TimePassesIsEnabled)
+        if (llvm::TimePassesIsEnabled) {
+          LLVMIRGenerationRefCount += 1;
+          if (LLVMIRGenerationRefCount == 1)
             LLVMIRGeneration.startTimer();
+        }
 
         Gen->HandleTranslationUnit(C);
 
-        if (llvm::TimePassesIsEnabled)
+        if (llvm::TimePassesIsEnabled) {
+          LLVMIRGenerationRefCount -= 1;
+          if (LLVMIRGenerationRefCount == 0)
             LLVMIRGeneration.stopTimer();
         }
+      }
 
       // Silently ignore if we weren't initialized for some reason.
       if (!getModule())
         return;
 
       // Install an inline asm handler so that diagnostics get printed through
       // our diagnostics hooks.
       LLVMContext &Ctx = getModule()->getContext();