[PATCH] D19243: [CodeGen] Move some CodeGenPGO stuff out of CodeGenFunction.h

Justin Bogner via cfe-commits cfe-commits at lists.llvm.org
Sun May 1 17:45:56 PDT 2016


Vedant Kumar <vsk at apple.com> writes:
> vsk created this revision.
> vsk added a reviewer: bogner.
> vsk added a subscriber: cfe-commits.
>
> Cons: 1 extra malloc per CodeGenFunction instantiation.
>
> Pros: This makes incremental builds noticeably faster, especially on
> my laptop. E.g with this patch there's no need to re-compile
> clang/lib/CodeGen/CGCUDARuntime.cpp InstrProfReader.h changes.

I doubt this is worthwhile. Mildly opposed.

If you do do this, use a unique_ptr rather than raw new/delete.

> http://reviews.llvm.org/D19243
>
> Files:
>   lib/CodeGen/CGBlocks.cpp
>   lib/CodeGen/CGCall.cpp
>   lib/CodeGen/CGExpr.cpp
>   lib/CodeGen/CGObjC.cpp
>   lib/CodeGen/CGStmt.cpp
>   lib/CodeGen/CGStmtOpenMP.cpp
>   lib/CodeGen/CodeGenFunction.cpp
>   lib/CodeGen/CodeGenFunction.h
>   lib/CodeGen/CodeGenPGO.cpp
>
> Index: lib/CodeGen/CodeGenPGO.cpp
> ===================================================================
> --- lib/CodeGen/CodeGenPGO.cpp
> +++ lib/CodeGen/CodeGenPGO.cpp
> @@ -878,12 +878,33 @@
>  
>  llvm::MDNode *CodeGenFunction::createProfileWeightsForLoop(const Stmt *Cond,
>                                                             uint64_t LoopCount) {
> -  if (!PGO.haveRegionCounts())
> +  if (!PGO->haveRegionCounts())
>      return nullptr;
> -  Optional<uint64_t> CondCount = PGO.getStmtCount(Cond);
> +  Optional<uint64_t> CondCount = PGO->getStmtCount(Cond);
>    assert(CondCount.hasValue() && "missing expected loop condition count");
>    if (*CondCount == 0)
>      return nullptr;
>    return createProfileWeights(LoopCount,
>                                std::max(*CondCount, LoopCount) - LoopCount);
>  }
> +
> +void CodeGenFunction::incrementProfileCounter(const Stmt *S) {
> +  if (CGM.getCodeGenOpts().hasProfileClangInstr())
> +    PGO->emitCounterIncrement(Builder, S);
> +  PGO->setCurrentStmt(S);
> +}
> +
> +uint64_t CodeGenFunction::getProfileCount(const Stmt *S) {
> +  Optional<uint64_t> Count = PGO->getStmtCount(S);
> +  if (!Count.hasValue())
> +    return 0;
> +  return *Count;
> +}
> +
> +void CodeGenFunction::setCurrentProfileCount(uint64_t Count) {
> +  PGO->setCurrentRegionCount(Count);
> +}
> +
> +uint64_t CodeGenFunction::getCurrentProfileCount() {
> +  return PGO->getCurrentRegionCount();
> +}
> Index: lib/CodeGen/CodeGenFunction.h
> ===================================================================
> --- lib/CodeGen/CodeGenFunction.h
> +++ lib/CodeGen/CodeGenFunction.h
> @@ -19,7 +19,6 @@
>  #include "CGLoopInfo.h"
>  #include "CGValue.h"
>  #include "CodeGenModule.h"
> -#include "CodeGenPGO.h"
>  #include "EHScopeStack.h"
>  #include "clang/AST/CharUnits.h"
>  #include "clang/AST/ExprCXX.h"
> @@ -76,6 +75,7 @@
>  class ObjCAutoreleasePoolStmt;
>  
>  namespace CodeGen {
> +class CodeGenPGO;
>  class CodeGenTypes;
>  class CGFunctionInfo;
>  class CGRecordLayout;
> @@ -692,7 +692,7 @@
>    /// block through the normal cleanup handling code (if any) and then
>    /// on to \arg Dest.
>    void EmitBranchThroughCleanup(JumpDest Dest);
> -  
> +
>    /// isObviouslyBranchWithoutCleanups - Return true if a branch to the
>    /// specified destination obviously has no cleanups to run.  'false' is always
>    /// a conservatively correct answer for this method.
> @@ -900,7 +900,7 @@
>        if (Data.isValid()) Data.unbind(CGF);
>      }
>    };
> -  
> +
>  private:
>    CGDebugInfo *DebugInfo;
>    bool DisableDebugInfo;
> @@ -943,7 +943,7 @@
>    };
>    SmallVector<BreakContinue, 8> BreakContinueStack;
>  
> -  CodeGenPGO PGO;
> +  CodeGenPGO *PGO;
>  
>    /// Calculate branch weights appropriate for PGO data
>    llvm::MDNode *createProfileWeights(uint64_t TrueCount, uint64_t FalseCount);
> @@ -953,30 +953,17 @@
>  
>  public:
>    /// Increment the profiler's counter for the given statement.
> -  void incrementProfileCounter(const Stmt *S) {
> -    if (CGM.getCodeGenOpts().hasProfileClangInstr())
> -      PGO.emitCounterIncrement(Builder, S);
> -    PGO.setCurrentStmt(S);
> -  }
> +  void incrementProfileCounter(const Stmt *S);
>  
>    /// Get the profiler's count for the given statement.
> -  uint64_t getProfileCount(const Stmt *S) {
> -    Optional<uint64_t> Count = PGO.getStmtCount(S);
> -    if (!Count.hasValue())
> -      return 0;
> -    return *Count;
> -  }
> +  uint64_t getProfileCount(const Stmt *S);
>  
>    /// Set the profiler's current count.
> -  void setCurrentProfileCount(uint64_t Count) {
> -    PGO.setCurrentRegionCount(Count);
> -  }
> +  void setCurrentProfileCount(uint64_t Count);
>  
>    /// Get the profiler's current count. This is generally the count for the most
>    /// recently incremented counter.
> -  uint64_t getCurrentProfileCount() {
> -    return PGO.getCurrentRegionCount();
> -  }
> +  uint64_t getCurrentProfileCount();
>  
>  private:
>  
> Index: lib/CodeGen/CodeGenFunction.cpp
> ===================================================================
> --- lib/CodeGen/CodeGenFunction.cpp
> +++ lib/CodeGen/CodeGenFunction.cpp
> @@ -52,7 +52,7 @@
>        ExceptionSlot(nullptr), EHSelectorSlot(nullptr),
>        DebugInfo(CGM.getModuleDebugInfo()),
>        DisableDebugInfo(false), DidCallStackSave(false), IndirectBranch(nullptr),
> -      PGO(cgm), SwitchInsn(nullptr), SwitchWeights(nullptr),
> +      PGO(new CodeGenPGO(cgm)), SwitchInsn(nullptr), SwitchWeights(nullptr),
>        CaseRangeBlock(nullptr), UnreachableBlock(nullptr), NumReturnExprs(0),
>        NumSimpleReturnExprs(0), CXXABIThisDecl(nullptr),
>        CXXABIThisValue(nullptr), CXXThisValue(nullptr),
> @@ -94,6 +94,8 @@
>    if (getLangOpts().OpenMP) {
>      CGM.getOpenMPRuntime().functionFinished(*this);
>    }
> +
> +  delete PGO;
>  }
>  
>  CharUnits CodeGenFunction::getNaturalPointeeTypeAlignment(QualType T,
> @@ -988,7 +990,7 @@
>    StartFunction(GD, ResTy, Fn, FnInfo, Args, Loc, BodyRange.getBegin());
>  
>    // Generate the body of the function.
> -  PGO.assignRegionCounters(GD, CurFn);
> +  PGO->assignRegionCounters(GD, CurFn);
>    if (isa<CXXDestructorDecl>(FD))
>      EmitDestructorBody(Args);
>    else if (isa<CXXConstructorDecl>(FD))
> Index: lib/CodeGen/CGStmtOpenMP.cpp
> ===================================================================
> --- lib/CodeGen/CGStmtOpenMP.cpp
> +++ lib/CodeGen/CGStmtOpenMP.cpp
> @@ -15,6 +15,7 @@
>  #include "CGOpenMPRuntime.h"
>  #include "CodeGenFunction.h"
>  #include "CodeGenModule.h"
> +#include "CodeGenPGO.h"
>  #include "TargetInfo.h"
>  #include "clang/AST/Stmt.h"
>  #include "clang/AST/StmtOpenMP.h"
> @@ -259,7 +260,7 @@
>      ++I;
>    }
>  
> -  PGO.assignRegionCounters(GlobalDecl(CD), F);
> +  PGO->assignRegionCounters(GlobalDecl(CD), F);
>    CapturedStmtInfo->EmitBody(*this, CD->getBody());
>    FinishFunction(CD->getBodyRBrace());
>  
> Index: lib/CodeGen/CGStmt.cpp
> ===================================================================
> --- lib/CodeGen/CGStmt.cpp
> +++ lib/CodeGen/CGStmt.cpp
> @@ -14,6 +14,7 @@
>  #include "CodeGenFunction.h"
>  #include "CGDebugInfo.h"
>  #include "CodeGenModule.h"
> +#include "CodeGenPGO.h"
>  #include "TargetInfo.h"
>  #include "clang/AST/StmtVisitor.h"
>  #include "clang/Basic/Builtins.h"
> @@ -47,7 +48,7 @@
>  
>  void CodeGenFunction::EmitStmt(const Stmt *S) {
>    assert(S && "Null statement?");
> -  PGO.setCurrentStmt(S);
> +  PGO->setCurrentStmt(S);
>  
>    // These statements have their own debug info handling.
>    if (EmitSimpleStmt(S))
> @@ -1488,7 +1489,7 @@
>    // failure.
>    llvm::BasicBlock *DefaultBlock = createBasicBlock("sw.default");
>    SwitchInsn = Builder.CreateSwitch(CondV, DefaultBlock);
> -  if (PGO.haveRegionCounts()) {
> +  if (PGO->haveRegionCounts()) {
>      // Walk the SwitchCase list to find how many there are.
>      uint64_t DefaultCount = 0;
>      unsigned NumCases = 0;
> @@ -2197,7 +2198,7 @@
>      CXXThisValue = EmitLoadOfLValue(ThisLValue, Loc).getScalarVal();
>    }
>  
> -  PGO.assignRegionCounters(GlobalDecl(CD), F);
> +  PGO->assignRegionCounters(GlobalDecl(CD), F);
>    CapturedStmtInfo->EmitBody(*this, CD->getBody());
>    FinishFunction(CD->getBodyRBrace());
>  
> Index: lib/CodeGen/CGObjC.cpp
> ===================================================================
> --- lib/CodeGen/CGObjC.cpp
> +++ lib/CodeGen/CGObjC.cpp
> @@ -15,6 +15,7 @@
>  #include "CGObjCRuntime.h"
>  #include "CodeGenFunction.h"
>  #include "CodeGenModule.h"
> +#include "CodeGenPGO.h"
>  #include "TargetInfo.h"
>  #include "clang/AST/ASTContext.h"
>  #include "clang/AST/DeclObjC.h"
> @@ -557,7 +558,7 @@
>  /// its pointer, name, and types registered in the class struture.
>  void CodeGenFunction::GenerateObjCMethod(const ObjCMethodDecl *OMD) {
>    StartObjCMethod(OMD, OMD->getClassInterface());
> -  PGO.assignRegionCounters(GlobalDecl(OMD), CurFn);
> +  PGO->assignRegionCounters(GlobalDecl(OMD), CurFn);
>    assert(isa<CompoundStmt>(OMD->getBody()));
>    incrementProfileCounter(OMD->getBody());
>    EmitCompoundStmtWithoutScope(*cast<CompoundStmt>(OMD->getBody()));
> Index: lib/CodeGen/CGExpr.cpp
> ===================================================================
> --- lib/CodeGen/CGExpr.cpp
> +++ lib/CodeGen/CGExpr.cpp
> @@ -19,6 +19,7 @@
>  #include "CGOpenMPRuntime.h"
>  #include "CGRecordLayout.h"
>  #include "CodeGenModule.h"
> +#include "CodeGenPGO.h"
>  #include "TargetInfo.h"
>  #include "clang/AST/ASTContext.h"
>  #include "clang/AST/Attr.h"
> @@ -106,7 +107,7 @@
>  /// EvaluateExprAsBool - Perform the usual unary conversions on the specified
>  /// expression and compare the result against zero, returning an Int1Ty value.
>  llvm::Value *CodeGenFunction::EvaluateExprAsBool(const Expr *E) {
> -  PGO.setCurrentStmt(E);
> +  PGO->setCurrentStmt(E);
>    if (const MemberPointerType *MPT = E->getType()->getAs<MemberPointerType>()) {
>      llvm::Value *MemPtr = EmitScalarExpr(E);
>      return CGM.getCXXABI().EmitMemberPointerIsNotNull(*this, MemPtr, MPT);
> Index: lib/CodeGen/CGCall.cpp
> ===================================================================
> --- lib/CodeGen/CGCall.cpp
> +++ lib/CodeGen/CGCall.cpp
> @@ -19,6 +19,7 @@
>  #include "CGCleanup.h"
>  #include "CodeGenFunction.h"
>  #include "CodeGenModule.h"
> +#include "CodeGenPGO.h"
>  #include "TargetInfo.h"
>  #include "clang/AST/Decl.h"
>  #include "clang/AST/DeclCXX.h"
> @@ -3944,8 +3945,8 @@
>    // For more details, see the comment before the definition of
>    // IPVK_IndirectCallTarget in InstrProfData.inc.
>    if (!CS.getCalledFunction())
> -    PGO.valueProfile(Builder, llvm::IPVK_IndirectCallTarget,
> -                     CS.getInstruction(), Callee);
> +    PGO->valueProfile(Builder, llvm::IPVK_IndirectCallTarget,
> +                      CS.getInstruction(), Callee);
>  
>    // In ObjC ARC mode with no ObjC ARC exception safety, tell the ARC
>    // optimizer it can aggressively ignore unwind edges.
> Index: lib/CodeGen/CGBlocks.cpp
> ===================================================================
> --- lib/CodeGen/CGBlocks.cpp
> +++ lib/CodeGen/CGBlocks.cpp
> @@ -16,6 +16,7 @@
>  #include "CGObjCRuntime.h"
>  #include "CodeGenFunction.h"
>  #include "CodeGenModule.h"
> +#include "CodeGenPGO.h"
>  #include "clang/AST/DeclObjC.h"
>  #include "llvm/ADT/SmallSet.h"
>  #include "llvm/IR/CallSite.h"
> @@ -1240,7 +1241,7 @@
>    if (IsLambdaConversionToBlock)
>      EmitLambdaBlockInvokeBody();
>    else {
> -    PGO.assignRegionCounters(GlobalDecl(blockDecl), fn);
> +    PGO->assignRegionCounters(GlobalDecl(blockDecl), fn);
>      incrementProfileCounter(blockDecl->getBody());
>      EmitStmt(blockDecl->getBody());
>    }


More information about the cfe-commits mailing list