diff --git a/llvm/include/llvm/Analysis/MemoryProfileInfo.h b/llvm/include/llvm/Analysis/MemoryProfileInfo.h index 571caf95f275d..be690a49767b4 100644 --- a/llvm/include/llvm/Analysis/MemoryProfileInfo.h +++ b/llvm/include/llvm/Analysis/MemoryProfileInfo.h @@ -59,6 +59,14 @@ LLVM_ABI std::string getAllocTypeAttributeString(AllocationType Type); /// True if the AllocTypes bitmask contains just a single type. LLVM_ABI bool hasSingleAllocType(uint8_t AllocTypes); +/// Removes any existing "ambiguous" memprof attribute. Called before we apply a +/// specific allocation type such as "cold", "notcold", or "hot". +LLVM_ABI void removeAnyExistingAmbiguousAttribute(CallBase *CB); + +/// Adds an "ambiguous" memprof attribute to call with a matched allocation +/// profile but that we haven't yet been able to disambiguate. +LLVM_ABI void addAmbiguousAttribute(CallBase *CB); + /// Class to build a trie of call stack contexts for a particular profiled /// allocation call, along with their associated allocation types. /// The allocation will be at the root of the trie, which is then used to diff --git a/llvm/lib/Analysis/MemoryProfileInfo.cpp b/llvm/lib/Analysis/MemoryProfileInfo.cpp index 0c1f8dbd1119a..92a5b6f378aab 100644 --- a/llvm/lib/Analysis/MemoryProfileInfo.cpp +++ b/llvm/lib/Analysis/MemoryProfileInfo.cpp @@ -54,6 +54,10 @@ cl::opt MinPercentMaxColdSize( "memprof-min-percent-max-cold-size", cl::init(100), cl::Hidden, cl::desc("Min percent of max cold bytes for critical cold context")); +LLVM_ABI cl::opt MemProfUseAmbiguousAttributes( + "memprof-ambiguous-attributes", cl::init(true), cl::Hidden, + cl::desc("Apply ambiguous memprof attribute to ambiguous allocations")); + } // end namespace llvm bool llvm::memprof::metadataIncludesAllContextSizeInfo() { @@ -125,6 +129,26 @@ bool llvm::memprof::hasSingleAllocType(uint8_t AllocTypes) { return NumAllocTypes == 1; } +void llvm::memprof::removeAnyExistingAmbiguousAttribute(CallBase *CB) { + if (!CB->hasFnAttr("memprof")) + return; + assert(CB->getFnAttr("memprof").getValueAsString() == "ambiguous"); + CB->removeFnAttr("memprof"); +} + +void llvm::memprof::addAmbiguousAttribute(CallBase *CB) { + if (!MemProfUseAmbiguousAttributes) + return; + // We may have an existing ambiguous attribute if we are reanalyzing + // after inlining. + if (CB->hasFnAttr("memprof")) { + assert(CB->getFnAttr("memprof").getValueAsString() == "ambiguous"); + } else { + auto A = llvm::Attribute::get(CB->getContext(), "memprof", "ambiguous"); + CB->addFnAttr(A); + } +} + void CallStackTrie::addCallStack( AllocationType AllocType, ArrayRef StackIds, std::vector ContextSizeInfo) { @@ -470,6 +494,9 @@ void CallStackTrie::addSingleAllocTypeAttribute(CallBase *CI, AllocationType AT, StringRef Descriptor) { auto AllocTypeString = getAllocTypeAttributeString(AT); auto A = llvm::Attribute::get(CI->getContext(), "memprof", AllocTypeString); + // After inlining we may be able to convert an existing ambiguous allocation + // to an unambiguous one. + removeAnyExistingAmbiguousAttribute(CI); CI->addFnAttr(A); if (MemProfReportHintedSizes) { std::vector ContextSizeInfo; @@ -529,6 +556,7 @@ bool CallStackTrie::buildAndAttachMIBMetadata(CallBase *CI) { assert(MIBCallStack.size() == 1 && "Should only be left with Alloc's location in stack"); CI->setMetadata(LLVMContext::MD_memprof, MDNode::get(Ctx, MIBNodes)); + addAmbiguousAttribute(CI); return true; } // If there exists corner case that CallStackTrie has one chain to leaf diff --git a/llvm/lib/Transforms/IPO/MemProfContextDisambiguation.cpp b/llvm/lib/Transforms/IPO/MemProfContextDisambiguation.cpp index ddb95a4184756..f9c12cf29ff58 100644 --- a/llvm/lib/Transforms/IPO/MemProfContextDisambiguation.cpp +++ b/llvm/lib/Transforms/IPO/MemProfContextDisambiguation.cpp @@ -3981,6 +3981,7 @@ void CallsiteContextGraph::identifyClones( void ModuleCallsiteContextGraph::updateAllocationCall( CallInfo &Call, AllocationType AllocType) { std::string AllocTypeString = getAllocTypeAttributeString(AllocType); + removeAnyExistingAmbiguousAttribute(cast(Call.call())); auto A = llvm::Attribute::get(Call.call()->getFunction()->getContext(), "memprof", AllocTypeString); cast(Call.call())->addFnAttr(A); @@ -5567,9 +5568,10 @@ bool MemProfContextDisambiguation::applyImport(Module &M) { auto *MemProfMD = I.getMetadata(LLVMContext::MD_memprof); // Include allocs that were already assigned a memprof function - // attribute in the statistics. - if (CB->getAttributes().hasFnAttr("memprof")) { - assert(!MemProfMD); + // attribute in the statistics. Only do this for those that do not have + // memprof metadata, since we add an "ambiguous" memprof attribute by + // default. + if (CB->getAttributes().hasFnAttr("memprof") && !MemProfMD) { CB->getAttributes().getFnAttr("memprof").getValueAsString() == "cold" ? AllocTypeColdThinBackend++ : AllocTypeNotColdThinBackend++; @@ -5642,6 +5644,7 @@ bool MemProfContextDisambiguation::applyImport(Module &M) { // clone J-1 (J==0 is the original clone and does not have a VMaps // entry). CBClone = cast((*VMaps[J - 1])[CB]); + removeAnyExistingAmbiguousAttribute(CBClone); CBClone->addFnAttr(A); ORE.emit(OptimizationRemark(DEBUG_TYPE, "MemprofAttribute", CBClone) << ore::NV("AllocationCall", CBClone) << " in clone " diff --git a/llvm/test/ThinLTO/X86/memprof-basic.ll b/llvm/test/ThinLTO/X86/memprof-basic.ll index 0ff0ce04452f1..537e1b8a26839 100644 --- a/llvm/test/ThinLTO/X86/memprof-basic.ll +++ b/llvm/test/ThinLTO/X86/memprof-basic.ll @@ -103,7 +103,9 @@ declare i32 @sleep() define internal ptr @_Z3barv() #0 !dbg !15 { entry: - %call = call ptr @_Znam(i64 0), !memprof !2, !callsite !7 + ;; Use an ambiguous attribute for this allocation, which is now added to such + ;; allocations during matching. It should not affect cloning. + %call = call ptr @_Znam(i64 0) #1, !memprof !2, !callsite !7 ret ptr null } @@ -125,6 +127,7 @@ entry: uselistorder ptr @_Z3foov, { 1, 0 } attributes #0 = { noinline optnone } +attributes #1 = { "memprof"="ambiguous" } !llvm.dbg.cu = !{!13} !llvm.module.flags = !{!20, !21} diff --git a/llvm/test/Transforms/PGOProfile/memprof.ll b/llvm/test/Transforms/PGOProfile/memprof.ll index c69d0311e0388..f6a89a8ceb86a 100644 --- a/llvm/test/Transforms/PGOProfile/memprof.ll +++ b/llvm/test/Transforms/PGOProfile/memprof.ll @@ -38,7 +38,7 @@ ; ALL-NOT: no profile data available for function ;; Using a memprof-only profile for memprof-use should only give memprof metadata -; RUN: opt < %s -passes='memprof-use' -pgo-warn-missing-function -S -memprof-print-match-info -stats 2>&1 | FileCheck %s --check-prefixes=MEMPROF,ALL,MEMPROFONLY,MEMPROFMATCHINFO,MEMPROFSTATS +; RUN: opt < %s -passes='memprof-use' -pgo-warn-missing-function -S -memprof-print-match-info -stats 2>&1 | FileCheck %s --check-prefixes=MEMPROF,ALL,MEMPROFONLY,MEMPROFMATCHINFO,MEMPROFSTATS,AMBIG ; There should not be any PGO metadata ; MEMPROFONLY-NOT: !prof @@ -51,10 +51,10 @@ ;; Test the same thing but by passing the memory profile through to a default ;; pipeline via -memory-profile-file=, which should cause the necessary field ;; of the PGOOptions structure to be populated with the profile filename. -; RUN: opt < %s -passes='default' -memory-profile-file=%t.memprofdata -pgo-warn-missing-function -S 2>&1 | FileCheck %s --check-prefixes=MEMPROF,ALL,MEMPROFONLY +; RUN: opt < %s -passes='default' -memory-profile-file=%t.memprofdata -pgo-warn-missing-function -S 2>&1 | FileCheck %s --check-prefixes=MEMPROF,ALL,MEMPROFONLY,AMBIG ;; Using a pgo+memprof profile for memprof-use should only give memprof metadata -; RUN: opt < %s -passes='memprof-use' -pgo-warn-missing-function -S 2>&1 | FileCheck %s --check-prefixes=MEMPROF,ALL,MEMPROFONLY +; RUN: opt < %s -passes='memprof-use' -pgo-warn-missing-function -S 2>&1 | FileCheck %s --check-prefixes=MEMPROF,ALL,MEMPROFONLY,AMBIG ;; Using a pgo-only profile for memprof-use should give an error ; RUN: not opt < %s -passes='memprof-use' -S 2>&1 | FileCheck %s --check-prefixes=MEMPROFWITHPGOONLY @@ -72,7 +72,7 @@ ;; Using a pgo+memprof profile for both memprof-use and pgo-instr-use should ;; give both memprof and pgo metadata. -; RUN: opt < %s -passes='pgo-instr-use,memprof-use' -pgo-test-profile-file=%t.pgomemprofdata -pgo-warn-missing-function -S 2>&1 | FileCheck %s --check-prefixes=MEMPROF,ALL,PGO +; RUN: opt < %s -passes='pgo-instr-use,memprof-use' -pgo-test-profile-file=%t.pgomemprofdata -pgo-warn-missing-function -S 2>&1 | FileCheck %s --check-prefixes=MEMPROF,ALL,PGO,AMBIG ;; Check that the total sizes are reported if requested. A message should be ;; emitted for the pruned context. Also check that remarks are emitted for the @@ -108,7 +108,11 @@ ;; However, with the same threshold, but hot hints not enabled, it should be ;; notcold again. -; RUN: opt < %s -passes='memprof-use' -pgo-warn-missing-function -S -memprof-min-ave-lifetime-access-density-hot-threshold=0 2>&1 | FileCheck %s --check-prefixes=MEMPROF,ALL +; RUN: opt < %s -passes='memprof-use' -pgo-warn-missing-function -S -memprof-min-ave-lifetime-access-density-hot-threshold=0 2>&1 | FileCheck %s --check-prefixes=MEMPROF,ALL,AMBIG + +;; Test that we don't get an ambiguous memprof attribute when +;; -memprof-ambiguous-attributes is disabled. +; RUN: opt < %s -passes='memprof-use' -pgo-warn-missing-function -S -memprof-ambiguous-attributes=false 2>&1 | FileCheck %s --check-prefixes=MEMPROF,ALL,NOAMBIG ; MEMPROFMATCHINFO: MemProf notcold context with id 1093248920606587996 has total profiled size 10 is matched with 1 frames ; MEMPROFMATCHINFO: MemProf notcold context with id 5725971306423925017 has total profiled size 10 is matched with 1 frames @@ -140,7 +144,7 @@ target triple = "x86_64-unknown-linux-gnu" ; PGO: !prof define dso_local noundef ptr @_Z3foov() #0 !dbg !10 { entry: - ; MEMPROF: call {{.*}} @_Znam{{.*}} !memprof ![[M1:[0-9]+]], !callsite ![[C1:[0-9]+]] + ; MEMPROF: call {{.*}} @_Znam{{.*}} #[[A0:[0-9]+]]{{.*}} !memprof ![[M1:[0-9]+]], !callsite ![[C1:[0-9]+]] ; MEMPROFNOCOLINFO: call {{.*}} @_Znam{{.*}} !memprof ![[M1:[0-9]+]], !callsite ![[C1:[0-9]+]] %call = call noalias noundef nonnull ptr @_Znam(i64 noundef 10) #6, !dbg !13 ret ptr %call, !dbg !14 @@ -364,6 +368,9 @@ for.end: ; preds = %for.cond ret i32 0, !dbg !103 } +;; We optionally apply an ambiguous memprof attribute to ambiguous allocations +; AMBIG: #[[A0]] = { builtin allocsize(0) "memprof"="ambiguous" } +; NOAMBIG: #[[A0]] = { builtin allocsize(0) } ; MEMPROF: #[[A1]] = { builtin allocsize(0) "memprof"="notcold" } ; MEMPROF: #[[A2]] = { builtin allocsize(0) "memprof"="cold" } ; MEMPROF: ![[M1]] = !{![[MIB1:[0-9]+]], ![[MIB2:[0-9]+]], ![[MIB3:[0-9]+]], ![[MIB4:[0-9]+]]} diff --git a/llvm/unittests/Analysis/MemoryProfileInfoTest.cpp b/llvm/unittests/Analysis/MemoryProfileInfoTest.cpp index d8457a30fd2f7..d1c0f643f5cd7 100644 --- a/llvm/unittests/Analysis/MemoryProfileInfoTest.cpp +++ b/llvm/unittests/Analysis/MemoryProfileInfoTest.cpp @@ -230,7 +230,8 @@ declare dso_local noalias noundef i8* @malloc(i64 noundef) CallBase *Call = findCall(*Func, "call"); Trie.buildAndAttachMIBMetadata(Call); - EXPECT_FALSE(Call->hasFnAttr("memprof")); + EXPECT_TRUE(Call->hasFnAttr("memprof")); + EXPECT_EQ(Call->getFnAttr("memprof").getValueAsString(), "ambiguous"); EXPECT_TRUE(Call->hasMetadata(LLVMContext::MD_memprof)); MDNode *MemProfMD = Call->getMetadata(LLVMContext::MD_memprof); ASSERT_EQ(MemProfMD->getNumOperands(), 2u); @@ -279,7 +280,8 @@ declare dso_local noalias noundef i8* @malloc(i64 noundef) CallBase *Call = findCall(*Func, "call"); Trie.buildAndAttachMIBMetadata(Call); - EXPECT_FALSE(Call->hasFnAttr("memprof")); + EXPECT_TRUE(Call->hasFnAttr("memprof")); + EXPECT_EQ(Call->getFnAttr("memprof").getValueAsString(), "ambiguous"); EXPECT_TRUE(Call->hasMetadata(LLVMContext::MD_memprof)); MDNode *MemProfMD = Call->getMetadata(LLVMContext::MD_memprof); ASSERT_EQ(MemProfMD->getNumOperands(), 2u); @@ -333,7 +335,8 @@ declare dso_local noalias noundef i8* @malloc(i64 noundef) CallBase *Call = findCall(*Func, "call"); Trie.buildAndAttachMIBMetadata(Call); - EXPECT_FALSE(Call->hasFnAttr("memprof")); + EXPECT_TRUE(Call->hasFnAttr("memprof")); + EXPECT_EQ(Call->getFnAttr("memprof").getValueAsString(), "ambiguous"); EXPECT_TRUE(Call->hasMetadata(LLVMContext::MD_memprof)); MDNode *MemProfMD = Call->getMetadata(LLVMContext::MD_memprof); ASSERT_EQ(MemProfMD->getNumOperands(), 2u); @@ -392,7 +395,8 @@ declare dso_local noalias noundef i8* @malloc(i64 noundef) CallBase *Call = findCall(*Func, "call"); Trie.buildAndAttachMIBMetadata(Call); - EXPECT_FALSE(Call->hasFnAttr("memprof")); + EXPECT_TRUE(Call->hasFnAttr("memprof")); + EXPECT_EQ(Call->getFnAttr("memprof").getValueAsString(), "ambiguous"); EXPECT_TRUE(Call->hasMetadata(LLVMContext::MD_memprof)); MDNode *MemProfMD = Call->getMetadata(LLVMContext::MD_memprof); ASSERT_EQ(MemProfMD->getNumOperands(), 2u); @@ -463,7 +467,8 @@ declare dso_local noalias noundef i8* @malloc(i64 noundef) ASSERT_NE(Call, nullptr); Trie.buildAndAttachMIBMetadata(Call); - EXPECT_FALSE(Call->hasFnAttr("memprof")); + EXPECT_TRUE(Call->hasFnAttr("memprof")); + EXPECT_EQ(Call->getFnAttr("memprof").getValueAsString(), "ambiguous"); EXPECT_TRUE(Call->hasMetadata(LLVMContext::MD_memprof)); MDNode *MemProfMD = Call->getMetadata(LLVMContext::MD_memprof); EXPECT_THAT(MemProfMD, MemprofMetadataEquals(ExpectedVals)); @@ -536,7 +541,8 @@ declare dso_local noalias noundef i8* @malloc(i64 noundef) // Restore original option value. MemProfKeepAllNotColdContexts = OrigMemProfKeepAllNotColdContexts; - EXPECT_FALSE(Call->hasFnAttr("memprof")); + EXPECT_TRUE(Call->hasFnAttr("memprof")); + EXPECT_EQ(Call->getFnAttr("memprof").getValueAsString(), "ambiguous"); EXPECT_TRUE(Call->hasMetadata(LLVMContext::MD_memprof)); MDNode *MemProfMD = Call->getMetadata(LLVMContext::MD_memprof); EXPECT_THAT(MemProfMD, MemprofMetadataEquals(ExpectedVals)); @@ -664,7 +670,8 @@ declare dso_local noalias noundef i8* @malloc(i64 noundef) // The hot allocations will be converted to NotCold and pruned as they // are unnecessary to determine how to clone the cold allocation. - EXPECT_FALSE(Call->hasFnAttr("memprof")); + EXPECT_TRUE(Call->hasFnAttr("memprof")); + EXPECT_EQ(Call->getFnAttr("memprof").getValueAsString(), "ambiguous"); EXPECT_TRUE(Call->hasMetadata(LLVMContext::MD_memprof)); MemProfMD = Call->getMetadata(LLVMContext::MD_memprof); ASSERT_EQ(MemProfMD->getNumOperands(), 2u);