Page MenuHomePhabricator

[MemCpyOpt] Enable MemorySSA by default
ClosedPublic

Authored by nikic on Jan 10 2021, 12:11 PM.

Details

Summary

This enables use of MemorySSA instead of MemDep in MemCpyOpt. To allow this without significant compile-time impact, the MemCpyOpt pass is moved directly before DSE (in the cases where this was not already the case) in order to avoid a duplicate MemorySSA construction.

With that done, this change is compile-time positive: http://llvm-compile-time-tracker.com/compare.php?from=f6f6f6375d1a4bced8a6e79a78726ab32b8dd879&to=3bef22406a62ae83712557038305cd135d1b1851&stat=instructions

Run-time impact is to be determined...

Diff Detail

Unit TestsFailed

TimeTest
90 msx64 debian > Clang.CodeGen::thinlto-distributed-newpm.ll
Script: -- : 'RUN: at line 6'; /mnt/disks/ssd0/agent/llvm-project/build/bin/opt -thinlto-bc -o /mnt/disks/ssd0/agent/llvm-project/build/tools/clang/test/CodeGen/Output/thinlto-distributed-newpm.ll.tmp.o /mnt/disks/ssd0/agent/llvm-project/clang/test/CodeGen/thinlto-distributed-newpm.ll
870 msx64 debian > Clang.CodeGenCXX::auto-var-init.cpp
Script: -- : 'RUN: at line 1'; /mnt/disks/ssd0/agent/llvm-project/build/bin/clang -cc1 -internal-isystem /mnt/disks/ssd0/agent/llvm-project/build/lib/clang/12.0.0/include -nostdsysteminc -std=c++14 -triple x86_64-unknown-unknown -fblocks /mnt/disks/ssd0/agent/llvm-project/clang/test/CodeGenCXX/auto-var-init.cpp -emit-llvm -o - | /mnt/disks/ssd0/agent/llvm-project/build/bin/FileCheck /mnt/disks/ssd0/agent/llvm-project/clang/test/CodeGenCXX/auto-var-init.cpp -check-prefixes=CHECK,CHECK-O0
220 msx64 windows > Clang.CodeGen::thinlto-distributed-newpm.ll
Script: -- : 'RUN: at line 6'; c:\ws\w16n2-1\llvm-project\premerge-checks\build\bin\opt.exe -thinlto-bc -o C:\ws\w16n2-1\llvm-project\premerge-checks\build\tools\clang\test\CodeGen\Output\thinlto-distributed-newpm.ll.tmp.o C:\ws\w16n2-1\llvm-project\premerge-checks\clang\test\CodeGen\thinlto-distributed-newpm.ll
1,170 msx64 windows > Clang.CodeGenCXX::auto-var-init.cpp
Script: -- : 'RUN: at line 1'; c:\ws\w16n2-1\llvm-project\premerge-checks\build\bin\clang.exe -cc1 -internal-isystem c:\ws\w16n2-1\llvm-project\premerge-checks\build\lib\clang\12.0.0\include -nostdsysteminc -std=c++14 -triple x86_64-unknown-unknown -fblocks C:\ws\w16n2-1\llvm-project\premerge-checks\clang\test\CodeGenCXX\auto-var-init.cpp -emit-llvm -o - | c:\ws\w16n2-1\llvm-project\premerge-checks\build\bin\filecheck.exe C:\ws\w16n2-1\llvm-project\premerge-checks\clang\test\CodeGenCXX\auto-var-init.cpp -check-prefixes=CHECK,CHECK-O0

Event Timeline

nikic created this revision.Jan 10 2021, 12:11 PM
nikic requested review of this revision.Jan 10 2021, 12:11 PM
Herald added a project: Restricted Project. · View Herald TranscriptJan 10 2021, 12:11 PM

A larger than expected codesize and runtime regression of Bullet benchmark (2.3%) for LTO. Maybe worth to analyse?

Could you test clang as well, and update test?

nikic updated this revision to Diff 316520.Jan 13 2021, 2:41 PM

Fix clang tests. Don't disable MemCpyOpt at O1 (previous version disabled it by accident).

I'm seeing crashes with this at stage3 in llvm::MemCpyOptPass::processMemCpyMemCpyDependence when invoking MSSA's Walker. I should have a repro next week.

lib/Transforms/IPO/PassManagerBuilder.cpp
496 ↗(On Diff #316520)

Move above comment.

asbirlea accepted this revision.Feb 17 2021, 1:24 PM

This looks good on my end, including run time testing.

This revision is now accepted and ready to land.Feb 17 2021, 1:24 PM
This revision was automatically updated to reflect the committed changes.
nikic marked an inline comment as done.
Herald added a project: Restricted Project. · View Herald TranscriptFeb 19 2021, 9:07 AM
Herald added a subscriber: cfe-commits. · View Herald Transcript

I ran into a crash in memcpyopt with this patch.
Running

opt -S -o - bbi-53212.ll -memcpyopt

yields

opt: ../include/llvm/Support/Casting.h:104: static bool llvm::isa_impl_cl<llvm::MemoryUse, const llvm::MemoryUseOrDef *>::doit(const From *) [To = llvm::MemoryUse, From = const llvm::MemoryUseOrDef *]: Assertion `Val && "isa<> used on a null pointer"' failed.
PLEASE submit a bug report to https://bugs.llvm.org/ and include the crash backtrace.
Stack dump:
0.	Program arguments: /data/repo/master/llvm/build-all/bin/opt -S -o - bbi-53212.ll -memcpyopt
 #0 0x00000000029461e3 llvm::sys::PrintStackTrace(llvm::raw_ostream&, int) (/data/repo/master/llvm/build-all/bin/opt+0x29461e3)
 #1 0x0000000002943e9e llvm::sys::RunSignalHandlers() (/data/repo/master/llvm/build-all/bin/opt+0x2943e9e)
 #2 0x00000000029466a6 SignalHandler(int) Signals.cpp:0:0
 #3 0x00007ff087446980 __restore_rt (/lib/x86_64-linux-gnu/libpthread.so.0+0x12980)
 #4 0x00007ff084517fb7 raise /build/glibc-S9d2JN/glibc-2.27/signal/../sysdeps/unix/sysv/linux/raise.c:51:0
 #5 0x00007ff084519921 abort /build/glibc-S9d2JN/glibc-2.27/stdlib/abort.c:81:0
 #6 0x00007ff08450948a __assert_fail_base /build/glibc-S9d2JN/glibc-2.27/assert/assert.c:89:0
 #7 0x00007ff084509502 (/lib/x86_64-linux-gnu/libc.so.6+0x30502)
 #8 0x00000000019f8722 llvm::MemoryUseOrDef::getDefiningAccess() const (/data/repo/master/llvm/build-all/bin/opt+0x19f8722)
 #9 0x0000000002766c6d llvm::MemCpyOptPass::processByValArgument(llvm::CallBase&, unsigned int) (/data/repo/master/llvm/build-all/bin/opt+0x2766c6d)
#10 0x0000000002767810 llvm::MemCpyOptPass::iterateOnFunction(llvm::Function&) (/data/repo/master/llvm/build-all/bin/opt+0x2767810)
#11 0x0000000002768280 llvm::MemCpyOptPass::runImpl(llvm::Function&, llvm::MemoryDependenceResults*, llvm::TargetLibraryInfo*, llvm::AAResults*, llvm::AssumptionCache*, llvm::DominatorTree*, llvm::MemorySSA*) (/data/repo/master/llvm/build-all/bin/opt+0x2768280)
#12 0x0000000002767c69 llvm::MemCpyOptPass::run(llvm::Function&, llvm::AnalysisManager<llvm::Function>&) (/data/repo/master/llvm/build-all/bin/opt+0x2767c69)
#13 0x0000000002bf031d llvm::detail::PassModel<llvm::Function, llvm::MemCpyOptPass, llvm::PreservedAnalyses, llvm::AnalysisManager<llvm::Function> >::run(llvm::Function&, llvm::AnalysisManager<llvm::Function>&) (/data/repo/master/llvm/build-all/bin/opt+0x2bf031d)
#14 0x00000000021720c9 llvm::PassManager<llvm::Function, llvm::AnalysisManager<llvm::Function> >::run(llvm::Function&, llvm::AnalysisManager<llvm::Function>&) (/data/repo/master/llvm/build-all/bin/opt+0x21720c9)
#15 0x0000000000a7cbfd llvm::detail::PassModel<llvm::Function, llvm::PassManager<llvm::Function, llvm::AnalysisManager<llvm::Function> >, llvm::PreservedAnalyses, llvm::AnalysisManager<llvm::Function> >::run(llvm::Function&, llvm::AnalysisManager<llvm::Function>&) (/data/repo/master/llvm/build-all/bin/opt+0xa7cbfd)
#16 0x00000000021768b6 llvm::ModuleToFunctionPassAdaptor::run(llvm::Module&, llvm::AnalysisManager<llvm::Module>&) (/data/repo/master/llvm/build-all/bin/opt+0x21768b6)
#17 0x000000000076c61d llvm::detail::PassModel<llvm::Module, llvm::ModuleToFunctionPassAdaptor, llvm::PreservedAnalyses, llvm::AnalysisManager<llvm::Module> >::run(llvm::Module&, llvm::AnalysisManager<llvm::Module>&) (/data/repo/master/llvm/build-all/bin/opt+0x76c61d)
#18 0x0000000002170f2b llvm::PassManager<llvm::Module, llvm::AnalysisManager<llvm::Module> >::run(llvm::Module&, llvm::AnalysisManager<llvm::Module>&) (/data/repo/master/llvm/build-all/bin/opt+0x2170f2b)
#19 0x0000000000764fe1 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, bool) (/data/repo/master/llvm/build-all/bin/opt+0x764fe1)
#20 0x0000000000776b32 main (/data/repo/master/llvm/build-all/bin/opt+0x776b32)
#21 0x00007ff0844fabf7 __libc_start_main /build/glibc-S9d2JN/glibc-2.27/csu/../csu/libc-start.c:344:0
#22 0x000000000075e76a _start (/data/repo/master/llvm/build-all/bin/opt+0x75e76a)

In my premerge testing, I see this fail which looks like the check string is incorrect:

command stderr:
C:\ws\w16e2-2\llvm-project\premerge-checks\llvm\test\Other\new-pm-lto-defaults.ll:84:21: error: CHECK-O23SZ-NEXT: expected string not found in input

; CHECK-O23SZ-NEXT: Running pass: InvalidateAnalysisPass<llvm::AAManager> on foo

                    ^

<stdin>:49:40: note: scanning from here

Running analysis: GlobalsAA on [module]

                                       ^

<stdin>:50:1: note: possible intended match here

Running pass: InvalidateAnalysisPass<class llvm::AAManager> on foo

https://reviews.llvm.org/harbormaster/unit/view/743692/ for D101640