[llvm] r269131 - Reapply r266477 and r266488
Sean Silva via llvm-commits
llvm-commits at lists.llvm.org
Tue May 10 16:17:27 PDT 2016
On Tue, May 10, 2016 at 3:03 PM, Easwaran Raman via llvm-commits <
llvm-commits at lists.llvm.org> wrote:
> Author: eraman
> Date: Tue May 10 17:03:23 2016
> New Revision: 269131
>
> URL: http://llvm.org/viewvc/llvm-project?rev=269131&view=rev
> Log:
> Reapply r266477 and r266488
>
> Added:
> llvm/trunk/unittests/ProfileData/ProfileSummaryTest.cpp
> - copied unchanged from r266618,
> llvm/trunk/unittests/ProfileData/ProfileSummaryTest.cpp
> Modified:
> llvm/trunk/include/llvm/ProfileData/ProfileCommon.h
> llvm/trunk/lib/Analysis/InlineCost.cpp
> llvm/trunk/lib/Analysis/LLVMBuild.txt
> llvm/trunk/lib/ProfileData/ProfileSummary.cpp
> llvm/trunk/test/Transforms/Inline/inline-cold-callee.ll
> llvm/trunk/test/Transforms/Inline/inline-hot-callee.ll
> llvm/trunk/unittests/ProfileData/CMakeLists.txt
>
> Modified: llvm/trunk/include/llvm/ProfileData/ProfileCommon.h
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/ProfileData/ProfileCommon.h?rev=269131&r1=269130&r2=269131&view=diff
>
> ==============================================================================
> --- llvm/trunk/include/llvm/ProfileData/ProfileCommon.h (original)
> +++ llvm/trunk/include/llvm/ProfileData/ProfileCommon.h Tue May 10
> 17:03:23 2016
> @@ -21,6 +21,8 @@
> #include <vector>
>
> #include "llvm/Support/Casting.h"
> +#include "llvm/Support/ManagedStatic.h"
> +#include "llvm/Support/Mutex.h"
>
> namespace llvm {
> class Function;
> @@ -35,6 +37,7 @@ class LLVMContext;
> class Metadata;
> class MDTuple;
> class MDNode;
> +class Module;
>
> inline const char *getHotSectionPrefix() { return ".hot"; }
> inline const char *getUnlikelySectionPrefix() { return ".unlikely"; }
> @@ -67,6 +70,14 @@ private:
> // appears in the profile. The map is kept sorted in the descending
> order of
> // counts.
> std::map<uint64_t, uint32_t, std::greater<uint64_t>> CountFrequencies;
> + // Compute profile summary for a module.
> + static ProfileSummary *computeProfileSummary(Module *M);
> + // Cache of last seen module and its profile summary.
> + static ManagedStatic<std::pair<Module *,
> std::unique_ptr<ProfileSummary>>>
> + CachedSummary;
> + // Mutex to access summary cache
> + static ManagedStatic<sys::SmartMutex<true>> CacheMutex;
>
Hi Easwaran,
In the LLVM core libraries, we don't use globals like this.
Can you please revert and redesign this to avoid the global? For example,
keeping it in the Module itself? I understand that there are some layering
issues here, and I'm glad to help with finding a nice solution, but putting
a global in the LLVM core libraries like this is not acceptable.
Thanks,
-- Sean Silva
> +
> protected:
> SummaryEntryVector DetailedSummary;
> std::vector<uint32_t> DetailedSummaryCutoffs;
> @@ -85,14 +96,12 @@ protected:
> : PSK(K), DetailedSummary(DetailedSummary), TotalCount(TotalCount),
> MaxCount(MaxCount), MaxFunctionCount(MaxFunctionCount),
> NumCounts(NumCounts), NumFunctions(NumFunctions) {}
> - ~ProfileSummary() = default;
> inline void addCount(uint64_t Count);
> /// \brief Return metadata specific to the profile format.
> /// Derived classes implement this method to return a vector of
> Metadata.
> virtual std::vector<Metadata *> getFormatSpecificMD(LLVMContext
> &Context) = 0;
> /// \brief Return detailed summary as metadata.
> Metadata *getDetailedSummaryMD(LLVMContext &Context);
> -
> public:
> static const int Scale = 1000000;
> Kind getKind() const { return PSK; }
> @@ -111,6 +120,10 @@ public:
> static ProfileSummary *getFromMD(Metadata *MD);
> uint32_t getNumFunctions() { return NumFunctions; }
> uint64_t getMaxFunctionCount() { return MaxFunctionCount; }
> + /// \brief Get profile summary associated with module \p M
> + static inline ProfileSummary *getProfileSummary(Module *M);
> + virtual ~ProfileSummary() = default;
> + virtual bool operator==(ProfileSummary &Other);
> };
>
> class InstrProfSummary final : public ProfileSummary {
> @@ -140,6 +153,7 @@ public:
> uint64_t getTotalCount() { return TotalCount; }
> uint64_t getMaxBlockCount() { return MaxCount; }
> uint64_t getMaxInternalBlockCount() { return MaxInternalBlockCount; }
> + bool operator==(ProfileSummary &Other) override;
> };
>
> class SampleProfileSummary final : public ProfileSummary {
> @@ -180,5 +194,24 @@ SummaryEntryVector &ProfileSummary::getD
> return DetailedSummary;
> }
>
> +ProfileSummary *ProfileSummary::getProfileSummary(Module *M) {
> + if (!M)
> + return nullptr;
> + sys::SmartScopedLock<true> Lock(*CacheMutex);
> + // Computing profile summary for a module involves parsing a fairly
> large
> + // metadata and could be expensive. We use a simple cache of the last
> seen
> + // module and its profile summary.
> + if (CachedSummary->first != M) {
> + auto *Summary = computeProfileSummary(M);
> + // Do not cache if the summary is empty. This is because a later pass
> + // (sample profile loader, for example) could attach the summary
> metadata on
> + // the module.
> + if (!Summary)
> + return nullptr;
> + CachedSummary->first = M;
> + CachedSummary->second.reset(Summary);
> + }
> + return CachedSummary->second.get();
> +}
> } // end namespace llvm
> #endif
>
> Modified: llvm/trunk/lib/Analysis/InlineCost.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Analysis/InlineCost.cpp?rev=269131&r1=269130&r2=269131&view=diff
>
> ==============================================================================
> --- llvm/trunk/lib/Analysis/InlineCost.cpp (original)
> +++ llvm/trunk/lib/Analysis/InlineCost.cpp Tue May 10 17:03:23 2016
> @@ -30,6 +30,7 @@
> #include "llvm/IR/InstVisitor.h"
> #include "llvm/IR/IntrinsicInst.h"
> #include "llvm/IR/Operator.h"
> +#include "llvm/ProfileData/ProfileCommon.h"
> #include "llvm/Support/Debug.h"
> #include "llvm/Support/raw_ostream.h"
>
> @@ -630,10 +631,11 @@ void CallAnalyzer::updateThreshold(CallS
> // a well-tuned heuristic based on *callsite* hotness and not callee
> hotness.
> uint64_t FunctionCount = 0, MaxFunctionCount = 0;
> bool HasPGOCounts = false;
> - if (Callee.getEntryCount() &&
> Callee.getParent()->getMaximumFunctionCount()) {
> + ProfileSummary *PS =
> ProfileSummary::getProfileSummary(Callee.getParent());
> + if (Callee.getEntryCount() && PS) {
> HasPGOCounts = true;
> FunctionCount = Callee.getEntryCount().getValue();
> - MaxFunctionCount =
> Callee.getParent()->getMaximumFunctionCount().getValue();
> + MaxFunctionCount = PS->getMaxFunctionCount();
> }
>
> // Listen to the inlinehint attribute or profile based hotness
> information
>
> Modified: llvm/trunk/lib/Analysis/LLVMBuild.txt
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Analysis/LLVMBuild.txt?rev=269131&r1=269130&r2=269131&view=diff
>
> ==============================================================================
> --- llvm/trunk/lib/Analysis/LLVMBuild.txt (original)
> +++ llvm/trunk/lib/Analysis/LLVMBuild.txt Tue May 10 17:03:23 2016
> @@ -19,4 +19,4 @@
> type = Library
> name = Analysis
> parent = Libraries
> -required_libraries = Core Support
> +required_libraries = Core ProfileData Support
>
> Modified: llvm/trunk/lib/ProfileData/ProfileSummary.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/ProfileData/ProfileSummary.cpp?rev=269131&r1=269130&r2=269131&view=diff
>
> ==============================================================================
> --- llvm/trunk/lib/ProfileData/ProfileSummary.cpp (original)
> +++ llvm/trunk/lib/ProfileData/ProfileSummary.cpp Tue May 10 17:03:23 2016
> @@ -15,6 +15,7 @@
> #include "llvm/IR/Constants.h"
> #include "llvm/IR/Function.h"
> #include "llvm/IR/Metadata.h"
> +#include "llvm/IR/Module.h"
> #include "llvm/IR/Type.h"
> #include "llvm/ProfileData/InstrProf.h"
> #include "llvm/ProfileData/ProfileCommon.h"
> @@ -32,6 +33,10 @@ const std::vector<uint32_t> ProfileSumma
> 900000, 950000, 990000, 999000, 999900, 999990, 999999});
> const char *ProfileSummary::KindStr[2] = {"InstrProf", "SampleProfile"};
>
> +ManagedStatic<std::pair<Module *, std::unique_ptr<ProfileSummary>>>
> + ProfileSummary::CachedSummary;
> +ManagedStatic<sys::SmartMutex<true>> ProfileSummary::CacheMutex;
> +
> void InstrProfSummary::addRecord(const InstrProfRecord &R) {
> // The first counter is not necessarily an entry count for IR
> // instrumentation profiles.
> @@ -86,6 +91,39 @@ void ProfileSummary::computeDetailedSumm
> }
> }
>
> +bool ProfileSummary::operator==(ProfileSummary &Other) {
> + if (getKind() != Other.getKind())
> + return false;
> + if (TotalCount != Other.TotalCount)
> + return false;
> + if (MaxCount != Other.MaxCount)
> + return false;
> + if (MaxFunctionCount != Other.MaxFunctionCount)
> + return false;
> + if (NumFunctions != Other.NumFunctions)
> + return false;
> + if (NumCounts != Other.NumCounts)
> + return false;
> + std::vector<ProfileSummaryEntry> DS1 = getDetailedSummary();
> + std::vector<ProfileSummaryEntry> DS2 = Other.getDetailedSummary();
> + auto CompareSummaryEntry = [](ProfileSummaryEntry &E1,
> + ProfileSummaryEntry &E2) {
> + return E1.Cutoff == E2.Cutoff && E1.MinCount == E2.MinCount &&
> + E1.NumCounts == E2.NumCounts;
> + };
> + if (!std::equal(DS1.begin(), DS1.end(), DS2.begin(),
> CompareSummaryEntry))
> + return false;
> + return true;
> +}
> +
> +bool InstrProfSummary::operator==(ProfileSummary &Other) {
> + InstrProfSummary *OtherIPS = dyn_cast<InstrProfSummary>(&Other);
> + if (!OtherIPS)
> + return false;
> + return MaxInternalBlockCount == OtherIPS->MaxInternalBlockCount &&
> + ProfileSummary::operator==(Other);
> +}
> +
> // Returns true if the function is a hot function.
> bool ProfileSummary::isFunctionHot(const Function *F) {
> // FIXME: update when summary data is stored in module's metadata.
> @@ -366,3 +404,9 @@ ProfileSummary *ProfileSummary::getFromM
> else
> return nullptr;
> }
> +
> +ProfileSummary *ProfileSummary::computeProfileSummary(Module *M) {
> + if (Metadata *MD = M->getProfileSummary())
> + return getFromMD(MD);
> + return nullptr;
> +}
>
> Modified: llvm/trunk/test/Transforms/Inline/inline-cold-callee.ll
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/Inline/inline-cold-callee.ll?rev=269131&r1=269130&r2=269131&view=diff
>
> ==============================================================================
> --- llvm/trunk/test/Transforms/Inline/inline-cold-callee.ll (original)
> +++ llvm/trunk/test/Transforms/Inline/inline-cold-callee.ll Tue May 10
> 17:03:23 2016
> @@ -5,7 +5,7 @@
> ; A callee with identical body does gets inlined because cost fits within
> the
> ; inline-threshold
>
> -define i32 @callee1(i32 %x) !prof !1 {
> +define i32 @callee1(i32 %x) !prof !21 {
> %x1 = add i32 %x, 1
> %x2 = add i32 %x1, 1
> %x3 = add i32 %x2, 1
> @@ -13,7 +13,7 @@ define i32 @callee1(i32 %x) !prof !1 {
> ret i32 %x3
> }
>
> -define i32 @callee2(i32 %x) !prof !2 {
> +define i32 @callee2(i32 %x) !prof !22 {
> ; CHECK-LABEL: @callee2(
> %x1 = add i32 %x, 1
> %x2 = add i32 %x1, 1
> @@ -22,7 +22,7 @@ define i32 @callee2(i32 %x) !prof !2 {
> ret i32 %x3
> }
>
> -define i32 @caller2(i32 %y1) !prof !2 {
> +define i32 @caller2(i32 %y1) !prof !22 {
> ; CHECK-LABEL: @caller2(
> ; CHECK: call i32 @callee2
> ; CHECK-NOT: call i32 @callee1
> @@ -32,8 +32,19 @@ define i32 @caller2(i32 %y1) !prof !2 {
> ret i32 %y3
> }
>
> -!llvm.module.flags = !{!0}
> -!0 = !{i32 1, !"MaxFunctionCount", i32 1000}
> -!1 = !{!"function_entry_count", i64 100}
> -!2 = !{!"function_entry_count", i64 1}
> -
> +!llvm.module.flags = !{!1}
> +!21 = !{!"function_entry_count", i64 100}
> +!22 = !{!"function_entry_count", i64 1}
> +
> +!1 = !{i32 1, !"ProfileSummary", !2}
> +!2 = !{!3, !4, !5, !6, !7, !8, !9, !10}
> +!3 = !{!"ProfileFormat", !"InstrProf"}
> +!4 = !{!"TotalCount", i64 10000}
> +!5 = !{!"MaxBlockCount", i64 1000}
> +!6 = !{!"MaxInternalBlockCount", i64 1}
> +!7 = !{!"MaxFunctionCount", i64 1000}
> +!8 = !{!"NumBlocks", i64 3}
> +!9 = !{!"NumFunctions", i64 3}
> +!10 = !{!"DetailedSummary", !11}
> +!11 = !{!12}
> +!12 = !{i32 10000, i64 0, i32 0}
>
> Modified: llvm/trunk/test/Transforms/Inline/inline-hot-callee.ll
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/Inline/inline-hot-callee.ll?rev=269131&r1=269130&r2=269131&view=diff
>
> ==============================================================================
> --- llvm/trunk/test/Transforms/Inline/inline-hot-callee.ll (original)
> +++ llvm/trunk/test/Transforms/Inline/inline-hot-callee.ll Tue May 10
> 17:03:23 2016
> @@ -5,7 +5,7 @@
> ; A cold callee with identical body does not get inlined because cost
> exceeds the
> ; inline-threshold
>
> -define i32 @callee1(i32 %x) !prof !1 {
> +define i32 @callee1(i32 %x) !prof !20 {
> %x1 = add i32 %x, 1
> %x2 = add i32 %x1, 1
> %x3 = add i32 %x2, 1
> @@ -13,7 +13,7 @@ define i32 @callee1(i32 %x) !prof !1 {
> ret i32 %x3
> }
>
> -define i32 @callee2(i32 %x) !prof !2 {
> +define i32 @callee2(i32 %x) !prof !21 {
> ; CHECK-LABEL: @callee2(
> %x1 = add i32 %x, 1
> %x2 = add i32 %x1, 1
> @@ -22,7 +22,7 @@ define i32 @callee2(i32 %x) !prof !2 {
> ret i32 %x3
> }
>
> -define i32 @caller2(i32 %y1) !prof !2 {
> +define i32 @caller2(i32 %y1) !prof !21 {
> ; CHECK-LABEL: @caller2(
> ; CHECK: call i32 @callee2
> ; CHECK-NOT: call i32 @callee1
> @@ -32,8 +32,19 @@ define i32 @caller2(i32 %y1) !prof !2 {
> ret i32 %y3
> }
>
> -!llvm.module.flags = !{!0}
> -!0 = !{i32 1, !"MaxFunctionCount", i32 10}
> -!1 = !{!"function_entry_count", i64 10}
> -!2 = !{!"function_entry_count", i64 1}
> -
> +!llvm.module.flags = !{!1}
> +!20 = !{!"function_entry_count", i64 10}
> +!21 = !{!"function_entry_count", i64 1}
> +
> +!1 = !{i32 1, !"ProfileSummary", !2}
> +!2 = !{!3, !4, !5, !6, !7, !8, !9, !10}
> +!3 = !{!"ProfileFormat", !"InstrProf"}
> +!4 = !{!"TotalCount", i64 10000}
> +!5 = !{!"MaxBlockCount", i64 10}
> +!6 = !{!"MaxInternalBlockCount", i64 1}
> +!7 = !{!"MaxFunctionCount", i64 10}
> +!8 = !{!"NumBlocks", i64 3}
> +!9 = !{!"NumFunctions", i64 3}
> +!10 = !{!"DetailedSummary", !11}
> +!11 = !{!12}
> +!12 = !{i32 10000, i64 0, i32 0}
>
> Modified: llvm/trunk/unittests/ProfileData/CMakeLists.txt
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/ProfileData/CMakeLists.txt?rev=269131&r1=269130&r2=269131&view=diff
>
> ==============================================================================
> --- llvm/trunk/unittests/ProfileData/CMakeLists.txt (original)
> +++ llvm/trunk/unittests/ProfileData/CMakeLists.txt Tue May 10 17:03:23
> 2016
> @@ -8,5 +8,6 @@ set(LLVM_LINK_COMPONENTS
> add_llvm_unittest(ProfileDataTests
> CoverageMappingTest.cpp
> InstrProfTest.cpp
> + ProfileSummaryTest.cpp
> SampleProfTest.cpp
> )
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160510/79a7c647/attachment.html>
More information about the llvm-commits
mailing list