[PATCH] D15158: [PGO] Instrument only base constructors and destructors.

Justin Bogner via cfe-commits cfe-commits at lists.llvm.org
Fri Dec 4 11:11:08 PST 2015 

Serge Pavlov <sepavloff at gmail.com> writes:
> sepavloff updated this revision to Diff 41744.
> sepavloff added a comment.
>
> Changes according to reviewer's notes.

LGTM.

>
> http://reviews.llvm.org/D15158
>
> Files:
>   lib/CodeGen/CGBlocks.cpp
>   lib/CodeGen/CGObjC.cpp
>   lib/CodeGen/CGStmt.cpp
>   lib/CodeGen/CGStmtOpenMP.cpp
>   lib/CodeGen/CodeGenFunction.cpp
>   lib/CodeGen/CodeGenPGO.cpp
>   lib/CodeGen/CodeGenPGO.h
>   test/Profile/cxx-structors.cpp
>   test/Profile/cxx-virtual-destructor-calls.cpp
>
> Index: test/Profile/cxx-virtual-destructor-calls.cpp
> ===================================================================
> --- test/Profile/cxx-virtual-destructor-calls.cpp
> +++ test/Profile/cxx-virtual-destructor-calls.cpp
> @@ -13,18 +13,25 @@
>    virtual ~B();
>  };
>  
> -// Complete dtor
> -// CHECK: @__llvm_profile_name__ZN1BD1Ev = private constant [9 x i8] c"_ZN1BD1Ev"
> +// Base dtor
> +// CHECK: @__llvm_profile_name__ZN1BD2Ev = private constant [9 x i8] c"_ZN1BD2Ev"
>  
> -// Deleting dtor
> -// CHECK: @__llvm_profile_name__ZN1BD0Ev = private constant [9 x i8] c"_ZN1BD0Ev"
> +// Complete dtor must not be instrumented
> +// CHECK-NOT: @__llvm_profile_name__ZN1BD1Ev = private constant [9 x i8] c"_ZN1BD1Ev"
>  
> -// Complete dtor counters and profile data
> -// CHECK: @__llvm_profile_counters__ZN1BD1Ev = private global [1 x i64] zeroinitializer
> -// CHECK: @__llvm_profile_data__ZN1BD1Ev =
> +// Deleting dtor must not be instrumented
> +// CHECK-NOT: @__llvm_profile_name__ZN1BD0Ev = private constant [9 x i8] c"_ZN1BD0Ev"
>  
> -// Deleting dtor counters and profile data
> -// CHECK: @__llvm_profile_counters__ZN1BD0Ev = private global [1 x i64] zeroinitializer
> -// CHECK: @__llvm_profile_data__ZN1BD0Ev =
> +// Base dtor counters and profile data
> +// CHECK: @__llvm_profile_counters__ZN1BD2Ev = private global [1 x i64] zeroinitializer
> +// CHECK: @__llvm_profile_data__ZN1BD2Ev =
> +
> +// Complete dtor counters and profile data must absent
> +// CHECK-NOT: @__llvm_profile_counters__ZN1BD1Ev = private global [1 x i64] zeroinitializer
> +// CHECK-NOT: @__llvm_profile_data__ZN1BD1Ev =
> +
> +// Deleting dtor counters and profile data must absent
> +// CHECK-NOT: @__llvm_profile_counters__ZN1BD0Ev = private global [1 x i64] zeroinitializer
> +// CHECK-NOT: @__llvm_profile_data__ZN1BD0Ev =
>  
>  B::~B() { }
> Index: test/Profile/cxx-structors.cpp
> ===================================================================
> --- /dev/null
> +++ test/Profile/cxx-structors.cpp
> @@ -0,0 +1,32 @@
> +// Tests for instrumentation of C++ constructors and destructors.
> +//
> +// RUN: %clang_cc1 -triple x86_64-apple-macosx10.11.0 -x c++ %s -o - -emit-llvm -fprofile-instr-generate | FileCheck %s
> +
> +struct Foo {
> +  Foo() {}
> +  Foo(int) {}
> +  ~Foo() {}
> +};
> +
> +struct Bar : public Foo {
> +  Bar() {}
> +  Bar(int x) : Foo(x) {}
> +  ~Bar();
> +};
> +
> +Foo foo;
> +Foo foo2(1);
> +Bar bar;
> +
> +// Profile data for complete constructors and destructors must absent.
> +
> +// CHECK-NOT: @__llvm_profile_name__ZN3FooC1Ev
> +// CHECK-NOT: @__llvm_profile_name__ZN3FooC1Ei
> +// CHECK-NOT: @__llvm_profile_name__ZN3FooD1Ev
> +// CHECK-NOT: @__llvm_profile_name__ZN3BarC1Ev
> +// CHECK-NOT: @__llvm_profile_name__ZN3BarD1Ev
> +// CHECK-NOT: @__llvm_profile_counters__ZN3FooD1Ev
> +// CHECK-NOT: @__llvm_profile_data__ZN3FooD1Ev
> +
> +int main() {
> +}
> Index: lib/CodeGen/CodeGenPGO.h
> ===================================================================
> --- lib/CodeGen/CodeGenPGO.h
> +++ lib/CodeGen/CodeGenPGO.h
> @@ -78,13 +78,11 @@
>        setCurrentRegionCount(*Count);
>    }
>  
> -  /// Check if we need to emit coverage mapping for a given declaration
> -  void checkGlobalDecl(GlobalDecl GD);
>    /// Assign counters to regions and configure them for PGO of a given
>    /// function. Does nothing if instrumentation is not enabled and either
>    /// generates global variables or associates PGO data with each of the
>    /// counters depending on whether we are generating or using instrumentation.
> -  void assignRegionCounters(const Decl *D, llvm::Function *Fn);
> +  void assignRegionCounters(GlobalDecl GD, llvm::Function *Fn);
>    /// Emit a coverage mapping range with a counter zero
>    /// for an unused declaration.
>    void emitEmptyCounterMapping(const Decl *D, StringRef FuncName,
> Index: lib/CodeGen/CodeGenPGO.cpp
> ===================================================================
> --- lib/CodeGen/CodeGenPGO.cpp
> +++ lib/CodeGen/CodeGenPGO.cpp
> @@ -602,27 +602,24 @@
>    return endian::read<uint64_t, little, unaligned>(Result);
>  }
>  
> -void CodeGenPGO::checkGlobalDecl(GlobalDecl GD) {
> -  // Make sure we only emit coverage mapping for one constructor/destructor.
> -  // Clang emits several functions for the constructor and the destructor of
> -  // a class. Every function is instrumented, but we only want to provide
> -  // coverage for one of them. Because of that we only emit the coverage mapping
> -  // for the base constructor/destructor.
> -  if ((isa<CXXConstructorDecl>(GD.getDecl()) &&
> -       GD.getCtorType() != Ctor_Base) ||
> -      (isa<CXXDestructorDecl>(GD.getDecl()) &&
> -       GD.getDtorType() != Dtor_Base)) {
> -    SkipCoverageMapping = true;
> -  }
> -}
> -
> -void CodeGenPGO::assignRegionCounters(const Decl *D, llvm::Function *Fn) {
> +void CodeGenPGO::assignRegionCounters(GlobalDecl GD, llvm::Function *Fn) {
> +  const Decl *D = GD.getDecl();
>    bool InstrumentRegions = CGM.getCodeGenOpts().ProfileInstrGenerate;
>    llvm::IndexedInstrProfReader *PGOReader = CGM.getPGOReader();
>    if (!InstrumentRegions && !PGOReader)
>      return;
>    if (D->isImplicit())
>      return;
> +  // Constructors and destructors may be represented by several functions in IR.
> +  // If so, instrument only base variant, others are implemented by delegation
> +  // to the base one, it would be counted twice otherwise.
> +  if (CGM.getTarget().getCXXABI().hasConstructorVariants() &&
> +      ((isa<CXXConstructorDecl>(GD.getDecl()) &&
> +        GD.getCtorType() != Ctor_Base) ||
> +       (isa<CXXDestructorDecl>(GD.getDecl()) &&
> +        GD.getDtorType() != Dtor_Base))) {
> +      return;
> +  }
>    CGM.ClearUnusedCoverageMapping(D);
>    setFuncName(Fn);
>  
> Index: lib/CodeGen/CodeGenFunction.cpp
> ===================================================================
> --- lib/CodeGen/CodeGenFunction.cpp
> +++ lib/CodeGen/CodeGenFunction.cpp
> @@ -946,8 +946,7 @@
>    StartFunction(GD, ResTy, Fn, FnInfo, Args, Loc, BodyRange.getBegin());
>  
>    // Generate the body of the function.
> -  PGO.checkGlobalDecl(GD);
> -  PGO.assignRegionCounters(GD.getDecl(), 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
> @@ -141,7 +141,7 @@
>      ++Cnt, ++I;
>    }
>  
> -  PGO.assignRegionCounters(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
> @@ -2167,7 +2167,7 @@
>      CXXThisValue = EmitLoadOfLValue(ThisLValue, Loc).getScalarVal();
>    }
>  
> -  PGO.assignRegionCounters(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
> @@ -557,7 +557,7 @@
>  /// its pointer, name, and types registered in the class struture.
>  void CodeGenFunction::GenerateObjCMethod(const ObjCMethodDecl *OMD) {
>    StartObjCMethod(OMD, OMD->getClassInterface());
> -  PGO.assignRegionCounters(OMD, CurFn);
> +  PGO.assignRegionCounters(GlobalDecl(OMD), CurFn);
>    assert(isa<CompoundStmt>(OMD->getBody()));
>    incrementProfileCounter(OMD->getBody());
>    EmitCompoundStmtWithoutScope(*cast<CompoundStmt>(OMD->getBody()));
> Index: lib/CodeGen/CGBlocks.cpp
> ===================================================================
> --- lib/CodeGen/CGBlocks.cpp
> +++ lib/CodeGen/CGBlocks.cpp
> @@ -1241,7 +1241,7 @@
>    if (IsLambdaConversionToBlock)
>      EmitLambdaBlockInvokeBody();
>    else {
> -    PGO.assignRegionCounters(blockDecl, fn);
> +    PGO.assignRegionCounters(GlobalDecl(blockDecl), fn);
>      incrementProfileCounter(blockDecl->getBody());
>      EmitStmt(blockDecl->getBody());
>    }

More information about the cfe-commits mailing list