This is an archive of the discontinued LLVM Phabricator instance.

[Inline] Make sure the InlineAdvisor is correctly cleared.
ClosedPublic

Authored by mtrofin on Oct 11 2021, 4:46 PM.

Details

Summary

If another inlining session came after a ModuleInlinerWrapperPass, the
advisor alanysis would still be cached, but its Result would be cleared.
We need to clear both.

This addresses PR52118

Diff Detail

Event Timeline

mtrofin created this revision.Oct 11 2021, 4:46 PM
mtrofin requested review of this revision.Oct 11 2021, 4:46 PM
Herald added a project: Restricted Project. · View Herald TranscriptOct 11 2021, 4:46 PM
mtrofin edited the summary of this revision. (Show Details)Oct 11 2021, 4:48 PM
aeubanks added inline comments.Oct 11 2021, 5:04 PM
llvm/include/llvm/Analysis/InlineAdvisor.h
231–232

this seems like it's reimplementing the invalidation.
can we get rid of clear() and use

bool invalidate(Module &, const PreservedAnalyses &PA,
                                 ModuleAnalysisManager::Invalidator &) {
  // Check whether the analysis has been explicitly invalidated. Otherwise, it's
  // stateless and remains preserved.
  auto PAC = PA.getChecker<InlineAdvisorAnalysis>();
  return !PAC.preservedWhenStateless();
}

?

llvm/lib/Transforms/IPO/Inliner.cpp
1053

can we

auto PA = PreservedAnalyses::all();
PA.abandon<>();
return PA;

instead? and add a comment

mtrofin updated this revision to Diff 378869.Oct 11 2021, 9:26 PM
mtrofin marked 2 inline comments as done.
mtrofin edited the summary of this revision. (Show Details)

feedback

mtrofin added inline comments.Oct 11 2021, 9:31 PM
llvm/lib/Transforms/IPO/Inliner.cpp
1053

done. btw, the newpm tests (e.g. Other/new-pm-defaults, etc) need updating. I'll do it once everything else looks OK

(...which is really just me procrastinating)

aeubanks accepted this revision.Oct 12 2021, 9:41 AM
This revision is now accepted and ready to land.Oct 12 2021, 9:41 AM
mtrofin updated this revision to Diff 379099.Oct 12 2021, 10:13 AM

Updated PM tests

This revision was landed with ongoing or failed builds.Oct 12 2021, 10:43 AM
This revision was automatically updated to reflect the committed changes.