Skip to content

Commit

Permalink
[MemProf] Improve metadata cleanup in LTO backend (#113039)
Browse files Browse the repository at this point in the history
Previously we were attempting to remove the memprof-related metadata
when iterating through instructions in the LTO backend. However, we
missed some as there are a number of cases where we skip instructions,
or even entire functions. Simplify the cleanup and ensure all is removed
by doing a full sweep over all instructions after completing cloning.

This is largely NFC except with -memprof-report-hinted-sizes enabled,
because we were propagating and simplifying the metadata after inlining
in the LTO backend, which caused some stray messages as metadata was
re-converted to attributes.
  • Loading branch information
teresajohnson authored Oct 21, 2024
1 parent 900b636 commit 120e42d
Show file tree
Hide file tree
Showing 2 changed files with 21 additions and 7 deletions.
23 changes: 17 additions & 6 deletions llvm/lib/Transforms/IPO/MemProfContextDisambiguation.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4264,9 +4264,6 @@ bool MemProfContextDisambiguation::applyImport(Module &M) {
AllocVersionsThinBackend++;
if (!MaxAllocVersionsThinBackend)
MaxAllocVersionsThinBackend = 1;
// Remove any remaining callsite metadata and we can skip the rest of
// the handling for this instruction, since no cloning needed.
I.setMetadata(LLVMContext::MD_callsite, nullptr);
continue;
}

Expand Down Expand Up @@ -4419,16 +4416,30 @@ bool MemProfContextDisambiguation::applyImport(Module &M) {
CloneCallsite(Callsite->second, CB, CalledFunction);
}
}
// Memprof and callsite metadata on memory allocations no longer needed.
I.setMetadata(LLVMContext::MD_memprof, nullptr);
I.setMetadata(LLVMContext::MD_callsite, nullptr);
}
}

// Now do any promotion required for cloning.
performICP(M, FS->callsites(), VMaps, ICallAnalysisInfo, ORE);
}

// We skip some of the functions and instructions above, so remove all the
// metadata in a single sweep here.
for (auto &F : M) {
// We can skip memprof clones because createFunctionClones already strips
// the metadata from the newly created clones.
if (F.isDeclaration() || isMemProfClone(F))
continue;
for (auto &BB : F) {
for (auto &I : BB) {
if (!isa<CallBase>(I))
continue;
I.setMetadata(LLVMContext::MD_memprof, nullptr);
I.setMetadata(LLVMContext::MD_callsite, nullptr);
}
}
}

return Changed;
}

Expand Down
5 changes: 4 additions & 1 deletion llvm/test/ThinLTO/X86/memprof-icp.ll
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,10 @@
; RUN: -pass-remarks=. -save-temps \
; RUN: -o %t.noicp.out 2>&1 | FileCheck %s --implicit-check-not "created clone"

; RUN: llvm-dis %t.noicp.out.2.4.opt.bc -o - | FileCheck %s --implicit-check-not "_Z3fooR2B0j.memprof"
;; Verify that we did not do any cloning of the function with the indirect call
;; when memprof ICP is off. However, we should still have removed the callsite
;; metadata.
; RUN: llvm-dis %t.noicp.out.2.4.opt.bc -o - | FileCheck %s --implicit-check-not "_Z3fooR2B0j.memprof" --implicit-check-not "!callsite"

; REMARKS-MAIN: call in clone main assigned to call function clone _Z3fooR2B0j.memprof.1
; REMARKS-MAIN: call in clone main assigned to call function clone _Z3fooR2B0j.memprof.1
Expand Down

0 comments on commit 120e42d

Please sign in to comment.