This is an archive of the discontinued LLVM Phabricator instance.

[PM] Port memdep to the new pass manager.
ClosedPublic

Authored by chandlerc on Mar 8 2016, 10:05 AM.

Details

Summary

This is a fairly straightforward port to the new pass manager with one
exception. It removes a very questionable use of releaseMemory() in
the old pass to invalidate its caches between runs on a function.
I don't think this is really guaranteed to be safe. I've just used the
more direct port to the new PM to address this by nuking the results
object each time the pass runs. While this could cause some minor malloc
traffic increase, I don't expect the compile time performance hit to be
noticable, and it makes the correctness and other aspects of the pass
much easier to reason about.

That said I'm sending it out for review so that Mehdi can help me confirm that
there is no obvious and significant compile time regression here.

There is sadly very limited testing at this point as there are only two
tests of memdep, and both rely on GVN. I'll be porting GVN next and that
will exercise this heavily though.

Diff Detail

Event Timeline

chandlerc updated this revision to Diff 50050.Mar 8 2016, 10:05 AM
chandlerc retitled this revision from to [PM] Port memdep to the new pass manager..
chandlerc updated this object.
chandlerc added a reviewer: mehdi_amini.
chandlerc added a subscriber: llvm-commits.
bruno added a subscriber: bruno.Mar 9 2016, 4:35 PM

Hi Chandler,

This is great!
I could not see any compile time regressions while running -O3 against the test-suite. But got more improvements/regressions results in -O2:

Compile-time O2

Performance Regressions - Compile Time Δ
External/SPEC/CFP2006/470_lbm/470_lbm 1.84%

Performance Improvements - Compile Time Δ Previous Current σ
SingleSource/Benchmarks/Adobe-C++/functionobjects -2.57% 0.7201 0.7016 0.0060
SingleSource/Benchmarks/Shootout-C++/fibo -2.51% 0.7045 0.6868 0.0057
SingleSource/Benchmarks/Shootout-C++/moments -2.44% 0.5503 0.5369 0.0048
SingleSource/Benchmarks/Shootout-C++/methcall -1.38% 0.7526 0.7422 0.0009
SingleSource/Benchmarks/Misc-C++/bigfib -1.31% 1.2855 1.2687 0.0032
MultiSource/Benchmarks/Prolangs-C++/employ/employ -1.31% 0.8499 0.8388 0.0039
SingleSource/Benchmarks/CoyoteBench/fftbench -1.18% 1.0390 1.0267 0.0031
MultiSource/Benchmarks/Prolangs-C++/city/city -1.00% 5.3008 5.2477 0.0100

In our internal compile time testsuite I could only see some small improvements (no regressions).

One odd thing I noticed was execution performance improvements due to this patch (multisample=5), which is probably not expected? Likely noise? but I'll paste the results just in case you want to double check those:

Execution time -O3

Performance Improvements - Execution Time Δ Previous Current σ
SingleSource/Benchmarks/Polybench/linear-algebra/kernels/trmm/trmm -4.81% 0.7553 0.7190 0.0011

Execution time -O2

Performance Improvements - Execution Time Δ Previous Current σ
SingleSource/Benchmarks/Shootout-C++/methcall -2.63% 5.1823 5.0462 0.0391
External/SPEC/CINT2000/253_perlbmk/253_perlbmk -2.14%
SingleSource/Benchmarks/Polybench/stencils/fdtd-apml/fdtd-apml -1.35% 1.1386 1.1232 0.0051

This revision was automatically updated to reflect the committed changes.