"Betul Buyukkurt" <[email protected]> writes: >> "Betul Buyukkurt" <[email protected]> writes: >>>> Betul Buyukkurt <[email protected]> writes: >>>>> Hi bogner, dsule, davidxl, >>>>> >>>>> -fprofile-instr-generate does not emit counter increment intrinsics >>>>> for Dtor_Deleting and Dtor_Complete destructors with assigned >>>>> counters. This causes unnecessary [-Wprofile-instr-out-of-date] >>>>> warnings during profile-use runs even if the source has never been >>>>> modified since profile collection. >>>>> >>>>> http://reviews.llvm.org/D9861 >>>>> >>>>> Files: >>>>> lib/CodeGen/CGClass.cpp >>>>> test/Profile/cxx-virtual-destructor-calls.cpp >>>>> >>>>> Index: lib/CodeGen/CGClass.cpp >>>>> =================================================================== >>>>> --- lib/CodeGen/CGClass.cpp >>>>> +++ lib/CodeGen/CGClass.cpp >>>>> @@ -1366,6 +1366,10 @@ >>>>> const CXXDestructorDecl *Dtor = >>>>> cast<CXXDestructorDecl>(CurGD.getDecl()); >>>>> CXXDtorType DtorType = CurGD.getDtorType(); >>>>> >>>>> + Stmt *Body = Dtor->getBody(); >>>>> + if (Body) >>>>> + incrementProfileCounter(Body); >>>>> + >>>> >>>> I think it makes more sense to do this after the delegation and >>>> entering >>>> the try body, like we do in constructors, no? >>> >>> I've been observing plenty warnings emitted from C++ codes. Tracking the >>> issue I saw that the Dtor_Deleting and other Dtor types have counters >>> assigned to them but no counter increment code is emitted. This causes >>> no >>> profile data structures to be emitted for those functions as well. In >>> CodeGenPGO.cpp line 649, it says that: >>> >>> " 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. " >>> >>> I also see that the counter increment is currently done for base >>> destructor only. This patch removes all the >>> [-Wprofile-instr-out-of-date] >>> warnings from the spec2000/2006 benchmarks. >> >> Well, yes. You explained that in the commit message. What I mean is, I >> think this belongs slightly later in the function to match how we do it >> in the constructor, ie: >> >> from CodeGenFunction::EmitDestructorBody(FunctionArgList &Args): >>> // The call to operator delete in a deleting destructor happens >>> // outside of the function-try-block, which means it's always >>> // possible to delegate the destructor body to the complete >>> // destructor. Do so. >>> if (DtorType == Dtor_Deleting) { >>> EnterDtorCleanups(Dtor, Dtor_Deleting); >>> EmitCXXDestructorCall(Dtor, Dtor_Complete, /*ForVirtualBase=*/false, >>> /*Delegating=*/false, LoadCXXThis()); >>> PopCleanupBlock(); >>> return; >>> } > > Dtor_Deleting returns here. So the increment also has to be inside the > above if statement, if the other location were to be below where you > proposed. The above location would diminish the code duplication.
Hm, I guess I wasn't thinking clearly - in the case where we delegate the call, we should still instrument the calling function. Your change is correct - feel free to commit. However, could you please look at EmitConstructorBody as well? It does not emit a counter increment today in the case where it delegates from the complete constructor to the base constructor. I suspect that's another instance of this same problem. _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
