vsk created this revision. We don't assign profile counters to non-base constructors. That results in a loss coverage for constructors in classes with virtual bases, which are complete constructors.
Make an exception for non-base constructors in classes with virtual bases. I checked that we get a proper code coverage report for both tests. @Alex, after I finished drafting this patch I noticed that you recommended using IsConstructorDelegationValid(), but imo that predicate seems a little hacked-up. Wdyt of starting with the virtual bases case, and then checking if there's anything else left? https://reviews.llvm.org/D30131 Files: lib/CodeGen/CodeGenPGO.cpp test/Profile/Inputs/cxx-class.proftext test/Profile/cxx-class.cpp test/Profile/cxx-structors.cpp
Index: test/Profile/cxx-structors.cpp =================================================================== --- test/Profile/cxx-structors.cpp +++ test/Profile/cxx-structors.cpp @@ -16,12 +16,22 @@ ~Bar(); }; +struct Baz : virtual public Foo { + Baz() {} + Baz(int x) : Foo(x) {} + ~Baz(); +}; + Foo foo; Foo foo2(1); Bar bar; +Baz baz; +Baz baz2(1); // Profile data for complete constructors and destructors must absent. +// INSTR: @__profc__ZN3BazC1Ev = +// INSTR: @__profc__ZN3BazC1Ei = // INSTR: @__profc_main = // INSTR: @__profc__ZN3FooC2Ev = // INSTR: @__profc__ZN3FooD2Ev = Index: test/Profile/cxx-class.cpp =================================================================== --- test/Profile/cxx-class.cpp +++ test/Profile/cxx-class.cpp @@ -5,17 +5,21 @@ // RUN: FileCheck --input-file=%tgen -check-prefix=DTRGEN %s // RUN: FileCheck --input-file=%tgen -check-prefix=MTHGEN %s // RUN: FileCheck --input-file=%tgen -check-prefix=WRPGEN %s +// RUN: FileCheck --input-file=%tgen -check-prefix=VCTRGEN %s +// RUN: FileCheck --input-file=%tgen -check-prefix=VDTRGEN %s // RUN: llvm-profdata merge %S/Inputs/cxx-class.proftext -o %t.profdata // RUN: %clang_cc1 %s -o - -emit-llvm -fprofile-instrument-use-path=%t.profdata -triple %itanium_abi_triple > %tuse // RUN: FileCheck --input-file=%tuse -check-prefix=CTRUSE %s // RUN: FileCheck --input-file=%tuse -check-prefix=DTRUSE %s // RUN: FileCheck --input-file=%tuse -check-prefix=MTHUSE %s // RUN: FileCheck --input-file=%tuse -check-prefix=WRPUSE %s +// RUN: FileCheck --input-file=%tuse -check-prefix=VCTRUSE %s +// RUN: FileCheck --input-file=%tuse -check-prefix=VDTRUSE %s class Simple { - int Member; public: + int Member; // CTRGEN-LABEL: define {{.*}} @_ZN6SimpleC2Ei( // CTRUSE-LABEL: define {{.*}} @_ZN6SimpleC2Ei( // CTRGEN: store {{.*}} @[[SCC:__profc__ZN6SimpleC2Ei]], i64 0, i64 0 @@ -56,13 +60,43 @@ // MTHUSE: ![[SM1]] = !{!"branch_weights", i32 100, i32 2} }; +class Derived : virtual public Simple { +public: + // VCTRGEN-LABEL: define {{.*}} @_ZN7DerivedC1Ev( + // VCTRUSE-LABEL: define {{.*}} @_ZN7DerivedC1Ev( + // VCTRGEN: store {{.*}} @[[SCC:__profc__ZN7DerivedC1Ev]], i64 0, i64 0 + Derived() : Simple(0) { + // VCTRGEN: store {{.*}} @[[SCC]], i64 0, i64 1 + // VCTRUSE: br {{.*}} !prof ![[SC1:[0-9]+]] + if (Member) {} + // VCTRGEN-NOT: store {{.*}} @[[SCC]], + // VCTRUSE-NOT: br {{.*}} !prof ![0-9]+ + // VCTRUSE: ret + } + // VCTRUSE: ![[SC1]] = !{!"branch_weights", i32 100, i32 2} + + // VDTRGEN-LABEL: define {{.*}} @_ZN7DerivedD2Ev( + // VDTRUSE-LABEL: define {{.*}} @_ZN7DerivedD2Ev( + // VDTRGEN: store {{.*}} @[[SDC:__profc__ZN7DerivedD2Ev]], i64 0, i64 0 + ~Derived() { + // VDTRGEN: store {{.*}} @[[SDC]], i64 0, i64 1 + // VDTRUSE: br {{.*}} !prof ![[SD1:[0-9]+]] + if (Member) {} + // VDTRGEN-NOT: store {{.*}} @[[SDC]], + // VDTRUSE-NOT: br {{.*}} !prof ![0-9]+ + // VDTRUSE: ret + } + // VDTRUSE: ![[SD1]] = !{!"branch_weights", i32 100, i32 2} +}; + // WRPGEN-LABEL: define {{.*}} @_Z14simple_wrapperv( // WRPUSE-LABEL: define {{.*}} @_Z14simple_wrapperv( // WRPGEN: store {{.*}} @[[SWC:__profc__Z14simple_wrapperv]], i64 0, i64 0 void simple_wrapper() { // WRPGEN: store {{.*}} @[[SWC]], i64 0, i64 1 // WRPUSE: br {{.*}} !prof ![[SW1:[0-9]+]] for (int I = 0; I < 100; ++I) { + Derived d; Simple S(I); S.method(); } Index: test/Profile/Inputs/cxx-class.proftext =================================================================== --- test/Profile/Inputs/cxx-class.proftext +++ test/Profile/Inputs/cxx-class.proftext @@ -39,3 +39,14 @@ 100 99 +_ZN7DerivedC1Ev +10 +2 +100 +99 + +_ZN7DerivedD2Ev +10 +2 +100 +99 Index: lib/CodeGen/CodeGenPGO.cpp =================================================================== --- lib/CodeGen/CodeGenPGO.cpp +++ lib/CodeGen/CodeGenPGO.cpp @@ -627,7 +627,8 @@ // 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>(D) && GD.getCtorType() != Ctor_Base) || + ((isa<CXXConstructorDecl>(D) && GD.getCtorType() != Ctor_Base && + !cast<CXXConstructorDecl>(D)->getParent()->getNumVBases()) || (isa<CXXDestructorDecl>(D) && GD.getDtorType() != Dtor_Base))) { return; }
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits