Skip to content

Commit e40cabf

Browse files
[MemProf] Match function's summary and definition strictly (#83665)
Problem description: #81008 (comment) Solution: #81008 (comment) (choose plan2)
1 parent 26722f5 commit e40cabf

File tree

4 files changed

+416
-9
lines changed

4 files changed

+416
-9
lines changed

llvm/lib/Passes/PassBuilderPipelines.cpp

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -299,9 +299,7 @@ static cl::opt<bool> UseLoopVersioningLICM(
299299
cl::desc("Enable the experimental Loop Versioning LICM pass"));
300300

301301
namespace llvm {
302-
cl::opt<bool> EnableMemProfContextDisambiguation(
303-
"enable-memprof-context-disambiguation", cl::init(false), cl::Hidden,
304-
cl::ZeroOrMore, cl::desc("Enable MemProf context disambiguation"));
302+
extern cl::opt<bool> EnableMemProfContextDisambiguation;
305303

306304
extern cl::opt<bool> EnableInferAlignmentPass;
307305
} // namespace llvm

llvm/lib/Transforms/IPO/FunctionImport.cpp

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -163,6 +163,10 @@ static cl::opt<std::string> WorkloadDefinitions(
163163
"}"),
164164
cl::Hidden);
165165

166+
namespace llvm {
167+
extern cl::opt<bool> EnableMemProfContextDisambiguation;
168+
}
169+
166170
// Load lazily a module from \p FileName in \p Context.
167171
static std::unique_ptr<Module> loadFile(const std::string &FileName,
168172
LLVMContext &Context) {
@@ -1643,7 +1647,9 @@ Expected<bool> FunctionImporter::importFunctions(
16431647
if (Import) {
16441648
if (Error Err = F.materialize())
16451649
return std::move(Err);
1646-
if (EnableImportMetadata) {
1650+
// MemProf should match function's definition and summary,
1651+
// 'thinlto_src_module' is needed.
1652+
if (EnableImportMetadata || EnableMemProfContextDisambiguation) {
16471653
// Add 'thinlto_src_module' and 'thinlto_src_file' metadata for
16481654
// statistics and debugging.
16491655
F.setMetadata(
@@ -1693,7 +1699,7 @@ Expected<bool> FunctionImporter::importFunctions(
16931699
LLVM_DEBUG(dbgs() << "Is importing aliasee fn " << GO->getGUID() << " "
16941700
<< GO->getName() << " from "
16951701
<< SrcModule->getSourceFileName() << "\n");
1696-
if (EnableImportMetadata) {
1702+
if (EnableImportMetadata || EnableMemProfContextDisambiguation) {
16971703
// Add 'thinlto_src_module' and 'thinlto_src_file' metadata for
16981704
// statistics and debugging.
16991705
Fn->setMetadata(

llvm/lib/Transforms/IPO/MemProfContextDisambiguation.cpp

Lines changed: 20 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -122,6 +122,10 @@ static cl::opt<unsigned>
122122
"frames through tail calls."));
123123

124124
namespace llvm {
125+
cl::opt<bool> EnableMemProfContextDisambiguation(
126+
"enable-memprof-context-disambiguation", cl::init(false), cl::Hidden,
127+
cl::ZeroOrMore, cl::desc("Enable MemProf context disambiguation"));
128+
125129
// Indicate we are linking with an allocator that supports hot/cold operator
126130
// new interfaces.
127131
cl::opt<bool> SupportsHotColdNew(
@@ -3375,10 +3379,22 @@ bool MemProfContextDisambiguation::applyImport(Module &M) {
33753379

33763380
auto *GVSummary =
33773381
ImportSummary->findSummaryInModule(TheFnVI, M.getModuleIdentifier());
3378-
if (!GVSummary)
3379-
// Must have been imported, use the first summary (might be multiple if
3380-
// this was a linkonce_odr).
3381-
GVSummary = TheFnVI.getSummaryList().front().get();
3382+
if (!GVSummary) {
3383+
// Must have been imported, use the summary which matches the definition。
3384+
// (might be multiple if this was a linkonce_odr).
3385+
auto SrcModuleMD = F.getMetadata("thinlto_src_module");
3386+
assert(SrcModuleMD &&
3387+
"enable-import-metadata is needed to emit thinlto_src_module");
3388+
StringRef SrcModule =
3389+
dyn_cast<MDString>(SrcModuleMD->getOperand(0))->getString();
3390+
for (auto &GVS : TheFnVI.getSummaryList()) {
3391+
if (GVS->modulePath() == SrcModule) {
3392+
GVSummary = GVS.get();
3393+
break;
3394+
}
3395+
}
3396+
assert(GVSummary && GVSummary->modulePath() == SrcModule);
3397+
}
33823398

33833399
// If this was an imported alias skip it as we won't have the function
33843400
// summary, and it should be cloned in the original module.

0 commit comments

Comments
 (0)