This is an archive of the discontinued LLVM Phabricator instance.

[NFC][MLGO] Use LazyCallGraph::Node to track functions.
ClosedPublic

Authored by mtrofin on Jan 10 2022, 11:21 AM.

Details

Summary

This avoids the InlineAdvisor carrying the responsibility of deleting
Function objects. We use LazyCallGraph::Node objects instead, which are
stable in memory for the duration of the Module-wide performance of CGSCC
passes started under the same ModuleToPostOrderCGSCCPassAdaptor (which
is the case here)

Diff Detail

Event Timeline

mtrofin created this revision.Jan 10 2022, 11:21 AM
mtrofin requested review of this revision.Jan 10 2022, 11:21 AM
Herald added a project: Restricted Project. · View Herald TranscriptJan 10 2022, 11:21 AM
mtrofin updated this revision to Diff 398707.Jan 10 2022, 11:29 AM

removed supurious include

aeubanks added inline comments.Jan 10 2022, 11:33 AM
llvm/lib/Analysis/DevelopmentModeInlineAdvisor.cpp
408

AFAICT, this is only called in the constructor of DevelopmentModeMLInlineAdvisor where we shouldn't have any deleted functions. and isFunctionDeleted() is only referenced here. is isFunctionDeleted() still necessary?

mtrofin updated this revision to Diff 399045.Jan 11 2022, 12:32 PM

no need for isFunctionDeleted

mtrofin edited the summary of this revision. (Show Details)Jan 11 2022, 12:33 PM
mtrofin marked an inline comment as done.
mtrofin added inline comments.
llvm/lib/Analysis/DevelopmentModeInlineAdvisor.cpp
408

yup - thanks! removed it.

lg, thanks!

llvm/include/llvm/Analysis/MLInlineAdvisor.h
12

shouldn't need this anymore

nikic added inline comments.Jan 11 2022, 12:48 PM
llvm/include/llvm/Analysis/MLInlineAdvisor.h
77

Where is DeletedFunctions used now?

mtrofin marked 3 inline comments as done.Jan 11 2022, 12:51 PM
mtrofin added inline comments.
llvm/include/llvm/Analysis/MLInlineAdvisor.h
12

Right. I'll move it to the .cpp because i'm still using it just to populate some initial state (I want to yank it though if I figure out how)

77

Nowhere - thanks for the catch!

This revision was not accepted when it landed; it landed in state Needs Review.Jan 11 2022, 7:39 PM
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
mtrofin marked 2 inline comments as done.
fhahn added a subscriber: fhahn.Jan 12 2022, 9:46 AM

It looks like this commit may be causing sanitizer errors in the inliner. They started with this commit: https://lab.llvm.org/buildbot/#/builders/5/builds/17176

********************
Testing:  0.. 10.. 20.. 30.. 40.. 50.. 60.. 70..
FAIL: LLVM :: Transforms/Inline/nested-inline.ll (63479 of 81196)
******************** TEST 'LLVM :: Transforms/Inline/nested-inline.ll' FAILED ********************
Script:
--
: 'RUN: at line 1';   /b/sanitizer-x86_64-linux-fast/build/llvm_build_asan/bin/opt < /b/sanitizer-x86_64-linux-fast/build/llvm-project/llvm/test/Transforms/Inline/nested-inline.ll -inline -S | /b/sanitizer-x86_64-linux-fast/build/llvm_build_asan/bin/FileCheck /b/sanitizer-x86_64-linux-fast/build/llvm-project/llvm/test/Transforms/Inline/nested-inline.ll
: 'RUN: at line 2';   /b/sanitizer-x86_64-linux-fast/build/llvm_build_asan/bin/opt < /b/sanitizer-x86_64-linux-fast/build/llvm-project/llvm/test/Transforms/Inline/nested-inline.ll -passes='cgscc(inline)' -S | /b/sanitizer-x86_64-linux-fast/build/llvm_build_asan/bin/FileCheck /b/sanitizer-x86_64-linux-fast/build/llvm-project/llvm/test/Transforms/Inline/nested-inline.ll
: 'RUN: at line 3';   /b/sanitizer-x86_64-linux-fast/build/llvm_build_asan/bin/opt < /b/sanitizer-x86_64-linux-fast/build/llvm-project/llvm/test/Transforms/Inline/nested-inline.ll -passes='module-inline' -S | /b/sanitizer-x86_64-linux-fast/build/llvm_build_asan/bin/FileCheck /b/sanitizer-x86_64-linux-fast/build/llvm-project/llvm/test/Transforms/Inline/nested-inline.ll
--
Exit Code: 1
Command Output (stderr):
--
=================================================================
==52172==ERROR: LeakSanitizer: detected memory leaks
Direct leak of 41 byte(s) in 2 object(s) allocated from:
    #0 0x51ccc8d in operator new(unsigned long) /b/s/w/ir/cache/builder/src/third_party/llvm/compiler-rt/lib/asan/asan_new_delete.cpp:95:3
    #1 0xb01dfcc in Allocate /b/sanitizer-x86_64-linux-fast/build/llvm-project/llvm/include/llvm/Support/AllocatorBase.h:85:12
    #2 0xb01dfcc in allocateWithKey<llvm::MallocAllocator> /b/sanitizer-x86_64-linux-fast/build/llvm-project/llvm/include/llvm/ADT/StringMapEntry.h:50:32
    #3 0xb01dfcc in Create<llvm::MallocAllocator> /b/sanitizer-x86_64-linux-fast/build/llvm-project/llvm/include/llvm/ADT/StringMapEntry.h:122:17
    #4 0xb01dfcc in llvm::Value::setNameImpl(llvm::Twine const&) /b/sanitizer-x86_64-linux-fast/build/llvm-project/llvm/lib/IR/Value.cpp:356:18
    #5 0xb01e3bd in llvm::Value::setName(llvm::Twine const&) /b/sanitizer-x86_64-linux-fast/build/llvm-project/llvm/lib/IR/Value.cpp:377:3
    #6 0xadeecd9 in GlobalValue /b/sanitizer-x86_64-linux-fast/build/llvm-project/llvm/include/llvm/IR/GlobalValue.h:85:5
    #7 0xadeecd9 in GlobalObject /b/sanitizer-x86_64-linux-fast/build/llvm-project/llvm/include/llvm/IR/GlobalObject.h:46:9
    #8 0xadeecd9 in llvm::Function::Function(llvm::FunctionType*, llvm::GlobalValue::LinkageTypes, unsigned int, llvm::Twine const&, llvm::Module*) /b/sanitizer-x86_64-linux-fast/build/llvm-project/llvm/lib/IR/Function.cpp:383:7
    #9 0x99928d4 in Create /b/sanitizer-x86_64-linux-fast/build/llvm-project/llvm/include/llvm/IR/Function.h:142:16
    #10 0x99928d4 in llvm::LLParser::parseFunctionHeader(llvm::Function*&, bool) /b/sanitizer-x86_64-linux-fast/build/llvm-project/llvm/lib/AsmParser/LLParser.cpp:5644:8
    #11 0x9985a96 in llvm::LLParser::parseDefine() /b/sanitizer-x86_64-linux-fast/build/llvm-project/llvm/lib/AsmParser/LLParser.cpp:540:10
    #12 0x997dca8 in llvm::LLParser::parseTopLevelEntities() /b/sanitizer-x86_64-linux-fast/build/llvm-project/llvm/lib/AsmParser/LLParser.cpp:352:11
    #13 0x997d8eb in llvm::LLParser::Run(bool, llvm::function_ref<llvm::Optional<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > > (llvm::StringRef)>) /b/sanitizer-x86_64-linux-fast/build/llvm-project/llvm/lib/AsmParser/LLParser.cpp:81:10
    #14 0x9963a0c in parseAssemblyInto(llvm::MemoryBufferRef, llvm::Module*, llvm::ModuleSummaryIndex*, llvm::SMDiagnostic&, llvm::SlotMapping*, bool, llvm::function_ref<llvm::Optional<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > > (llvm::StringRef)>) /b/sanitizer-x86_64-linux-fast/build/llvm-project/llvm/lib/AsmParser/Parser.cpp:36:8
    #15 0x9963dd6 in parseAssemblyInto /b/sanitizer-x86_64-linux-fast/build/llvm-project/llvm/lib/AsmParser/Parser.cpp:43:10
    #16 0x9963dd6 in llvm::parseAssembly(llvm::MemoryBufferRef, llvm::SMDiagnostic&, llvm::LLVMContext&, llvm::SlotMapping*, llvm::function_ref<llvm::Optional<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > > (llvm::StringRef)>) /b/sanitizer-x86_64-linux-fast/build/llvm-project/llvm/lib/AsmParser/Parser.cpp:54:7
    #17 0xb7db7f8 in llvm::parseIR(llvm::MemoryBufferRef, llvm::SMDiagnostic&, llvm::LLVMContext&, llvm::function_ref<llvm::Optional<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > > (llvm::StringRef)>) /b/sanitizer-x86_64-linux-fast/build/llvm-project/llvm/lib/IRReader/IRReader.cpp:88:10
    #18 0xb7dcaee in llvm::parseIRFile(llvm::StringRef, llvm::SMDiagnostic&, llvm::LLVMContext&, llvm::function_ref<llvm::Optional<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > > (llvm::StringRef)>) /b/sanitizer-x86_64-linux-fast/build/llvm-project/llvm/lib/IRReader/IRReader.cpp:102:10
    #19 0x5227789 in main /b/sanitizer-x86_64-linux-fast/build/llvm-project/llvm/tools/opt/opt.cpp:628:9
    #20 0x7f853d78209a in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2409a) (BuildId: 18b9a9a8c523e5cfe5b5d946d605d09242f09798)
Indirect leak of 780 byte(s) in 1 object(s) allocated from:
    #0 0x51922a8 in calloc /b/s/w/ir/cache/builder/src/third_party/llvm/compiler-rt/lib/asan/asan_malloc_linux.cpp:77:3
    #1 0xc9aa451 in llvm::safe_calloc(unsigned long, unsigned long) /b/sanitizer-x86_64-linux-fast/build/llvm-project/llvm/include/llvm/Support/MemAlloc.h:40:18
    #2 0xc9ab9ad in llvm::StringMapImpl::RehashTable(unsigned int) /b/sanitizer-x86_64-linux-fast/build/llvm-project/llvm/lib/Support/StringMap.cpp:219:59
    #3 0xb02ef01 in insert /b/sanitizer-x86_64-linux-fast/build/llvm-project/llvm/include/llvm/ADT/StringMap.h:286:5
    #4 0xb02ef01 in llvm::ValueSymbolTable::reinsertValue(llvm::Value*) /b/sanitizer-x86_64-linux-fast/build/llvm-project/llvm/lib/IR/ValueSymbolTable.cpp:76:12
    #5 0xab97270 in void llvm::SymbolTableListTraits<llvm::Instruction>::setSymTabObject<llvm::Function*>(llvm::Function**, llvm::Function*) /b/sanitizer-x86_64-linux-fast/build/llvm-project/llvm/lib/IR/SymbolTableListTraitsImpl.h:62:16
    #6 0xade9c59 in llvm::SymbolTableListTraits<llvm::BasicBlock>::addNodeToList(llvm::BasicBlock*) /b/sanitizer-x86_64-linux-fast/build/llvm-project/llvm/lib/IR/SymbolTableListTraitsImpl.h:71:6
    #7 0xcb87e64 in insert /b/sanitizer-x86_64-linux-fast/build/llvm-project/llvm/include/llvm/ADT/ilist.h:229:11
    #8 0xcb87e64 in push_back /b/sanitizer-x86_64-linux-fast/build/llvm-project/llvm/include/llvm/ADT/ilist.h:313:33
    #9 0xcb87e64 in llvm::CloneAndPruneIntoFromInst(llvm::Function*, llvm::Function const*, llvm::Instruction const*, llvm::ValueMap<llvm::Value const*, llvm::WeakTrackingVH, llvm::ValueMapConfig<llvm::Value const*, llvm::sys::SmartMutex<false> > >&, bool, llvm::SmallVectorImpl<llvm::ReturnInst*>&, char const*, llvm::ClonedCodeInfo*) /b/sanitizer-x86_64-linux-fast/build/llvm-project/llvm/lib/Transforms/Utils/CloneFunction.cpp:548:34
    #10 0xcb94c99 in llvm::CloneAndPruneFunctionInto(llvm::Function*, llvm::Function const*, llvm::ValueMap<llvm::Value const*, llvm::WeakTrackingVH, llvm::ValueMapConfig<llvm::Value const*, llvm::sys::SmartMutex<false> > >&, bool, llvm::SmallVectorImpl<llvm::ReturnInst*>&, char const*, llvm::ClonedCodeInfo*) /b/sanitizer-x86_64-linux-fast/build/llvm-project/llvm/lib/Transforms/Utils/CloneFunction.cpp:782:3
    #11 0xcc66eda in llvm::InlineFunction(llvm::CallBase&, llvm::InlineFunctionInfo&, llvm::AAResults*, bool, llvm::Function*) /b/sanitizer-x86_64-linux-fast/build/llvm-project/llvm/lib/Transforms/Utils/InlineFunction.cpp:1949:5
    #12 0xd4f9e97 in llvm::ModuleInlinerPass::run(llvm::Module&, llvm::AnalysisManager<llvm::Module>&) /b/sanitizer-x86_64-linux-fast/build/llvm-project/llvm/lib/Transforms/IPO/ModuleInliner.cpp:252:11
    #13 0xd43c4f1 in llvm::detail::PassModel<llvm::Module, llvm::ModuleInlinerPass, llvm::PreservedAnalyses, llvm::AnalysisManager<llvm::Module> >::run(llvm::Module&, llvm::AnalysisManager<llvm::Module>&) /b/sanitizer-x86_64-linux-fast/build/llvm-project/llvm/include/llvm/IR/PassManagerInternal.h:88:17
    #14 0xafaf29a in llvm::PassManager<llvm::Module, llvm::AnalysisManager<llvm::Module> >::run(llvm::Module&, llvm::AnalysisManager<llvm::Module>&) /b/sanitizer-x86_64-linux-fast/build/llvm-project/llvm/include/llvm/IR/PassManager.h:525:21
    #15 0x51e2b4e in llvm::runPassPipeline(llvm::StringRef, llvm::Module&, llvm::TargetMachine*, llvm::TargetLibraryInfoImpl*, llvm::ToolOutputFile*, llvm::ToolOutputFile*, llvm::ToolOutputFile*, llvm::StringRef, llvm::ArrayRef<llvm::StringRef>, llvm::opt_tool::OutputKind, llvm::opt_tool::VerifierKind, bool, bool, bool, bool, bool) /b/sanitizer-x86_64-linux-fast/build/llvm-project/llvm/tools/opt/NewPMDriver.cpp:487:7
    #16 0x522d67a in main /b/sanitizer-x86_64-linux-fast/build/llvm-project/llvm/tools/opt/opt.cpp:818:12
    #17 0x7f853d78209a in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2409a) (BuildId: 18b9a9a8c523e5cfe5b5d946d605d09242f09798)

Sorry about that, looking

Sorry about that, looking

fixed in fe827a93f69ddea80308d63a6b54b17106779354

caused by code duplication with the module inliner. is it still in use?

mtrofin added a subscriber: kazu.Jan 12 2022, 10:07 AM

thanks, Arthur, for the quick fix!