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

2015-12-06 Thread Serge Pavlov via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL254876: [PGO] Instrument only base constructors and 
destructors. (authored by sepavloff).

Changed prior to commit:
  http://reviews.llvm.org/D15158?vs=41744=42011#toc

Repository:
  rL LLVM

http://reviews.llvm.org/D15158

Files:
  cfe/trunk/lib/CodeGen/CGBlocks.cpp
  cfe/trunk/lib/CodeGen/CGObjC.cpp
  cfe/trunk/lib/CodeGen/CGStmt.cpp
  cfe/trunk/lib/CodeGen/CGStmtOpenMP.cpp
  cfe/trunk/lib/CodeGen/CodeGenFunction.cpp
  cfe/trunk/lib/CodeGen/CodeGenPGO.cpp
  cfe/trunk/lib/CodeGen/CodeGenPGO.h
  cfe/trunk/test/Profile/cxx-structors.cpp
  cfe/trunk/test/Profile/cxx-virtual-destructor-calls.cpp

Index: cfe/trunk/test/Profile/cxx-virtual-destructor-calls.cpp
===
--- cfe/trunk/test/Profile/cxx-virtual-destructor-calls.cpp
+++ cfe/trunk/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 counters and profile data
-// CHECK: @__llvm_profile_counters__ZN1BD0Ev = private global [1 x i64] zeroinitializer
-// CHECK: @__llvm_profile_data__ZN1BD0Ev =
+// Deleting dtor must not be instrumented
+// CHECK-NOT: @__llvm_profile_name__ZN1BD0Ev = private constant [9 x i8] c"_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: cfe/trunk/test/Profile/cxx-structors.cpp
===
--- cfe/trunk/test/Profile/cxx-structors.cpp
+++ cfe/trunk/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: cfe/trunk/lib/CodeGen/CodeGenPGO.cpp
===
--- cfe/trunk/lib/CodeGen/CodeGenPGO.cpp
+++ cfe/trunk/lib/CodeGen/CodeGenPGO.cpp
@@ -605,27 +605,24 @@
   return endian::read(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(GD.getDecl()) &&
-   GD.getCtorType() != Ctor_Base) ||
-  (isa(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 

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

2015-12-04 Thread Justin Bogner via cfe-commits
Serge Pavlov  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(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 

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

2015-12-03 Thread Serge Pavlov via cfe-commits
sepavloff updated this revision to Diff 41744.
sepavloff added a comment.

Changes according to reviewer's notes.


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(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(GD.getDecl()) &&
-   GD.getCtorType() != Ctor_Base) ||
-  (isa(GD.getDecl()) &&
-   GD.getDtorType() != Dtor_Base)) {
-SkipCoverageMapping = true;
-  }
-}
-
-void CodeGenPGO::assignRegionCounters(const 

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

2015-12-03 Thread Serge Pavlov via cfe-commits
Thank you, Justin.

2015-12-03 5:37 GMT+06:00 Justin Bogner :

> Serge Pavlov  writes:
> > sepavloff created this revision.
> > sepavloff added a reviewer: bogner.
> > sepavloff added subscribers: cfe-commits, silvas.
> >
> > Constructors and destructors may be represented by several functions
> > in IR. Only the base structors correspond to source code, others
> > are small pieces of code and eventually call the base variant. In
> > this case instrumentation of non-base structors has little sense,
> > this fix remove it. Now profile data of a declaration correspond to
> > exactly one function in IR, it agrees with current logic of profile
> > data loading.
> >
> > This change fixes PR24996.
>
> This looks like the right thing to do. A couple of comments on the patch
> below.
>
> >
> > 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
> > +// @__llvm_profile_name__ZN1BD1Ev = private constant [9 x i8]
> c"_ZN1BD1Ev"
>
> I guess these should be CHECK-NOT instead of just comments.
>

Exactly. Fixed.


>
> >
> > -// 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
> > +// @__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
> > +// @__llvm_profile_counters__ZN1BD1Ev = private global [1 x i64]
> zeroinitializer
> > +// @__llvm_profile_data__ZN1BD1Ev =
> > +
> > +// Deleting dtor counters and profile data must absent
> > +// @__llvm_profile_counters__ZN1BD0Ev = private global [1 x i64]
> zeroinitializer
> > +// @__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.
> > -  

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

2015-12-02 Thread Justin Bogner via cfe-commits
Serge Pavlov  writes:
> sepavloff created this revision.
> sepavloff added a reviewer: bogner.
> sepavloff added subscribers: cfe-commits, silvas.
>
> Constructors and destructors may be represented by several functions
> in IR. Only the base structors correspond to source code, others
> are small pieces of code and eventually call the base variant. In
> this case instrumentation of non-base structors has little sense,
> this fix remove it. Now profile data of a declaration correspond to
> exactly one function in IR, it agrees with current logic of profile
> data loading.
>
> This change fixes PR24996.

This looks like the right thing to do. A couple of comments on the patch
below.

>
> 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
> +// @__llvm_profile_name__ZN1BD1Ev = private constant [9 x i8] c"_ZN1BD1Ev"

I guess these should be CHECK-NOT instead of just comments.

>  
> -// 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
> +// @__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
> +// @__llvm_profile_counters__ZN1BD1Ev = private global [1 x i64] 
> zeroinitializer
> +// @__llvm_profile_data__ZN1BD1Ev =
> +
> +// Deleting dtor counters and profile data must absent
> +// @__llvm_profile_counters__ZN1BD0Ev = private global [1 x i64] 
> zeroinitializer
> +// @__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
>