This is an archive of the discontinued LLVM Phabricator instance.

[Passes] Move ADCE before DSE & LICM.
ClosedPublic

Authored by fhahn on Sep 8 2020, 12:39 PM.

Details

Summary

The adjustment seems to have very little impact on optimizations.
The only binary change with -O3 MultiSource/SPEC2000/SPEC2006 on X86 is
in consumer-typeset and the size there actually decreases by -0.1%, with
not significant changes in the stats.

On its own, it is mildly positive in terms of compile-time, most likely
due to LICM & DSE having to process slightly less instructions. It
should also be unlikely that DSE/LICM make much new code dead.

http://llvm-compile-time-tracker.com/compare.php?from=df63eedef64d715ce1f31843f7de9c11fe1e597f&to=e3bdfcf94a9eeae6e006d010464f0c1b3550577d&stat=instructions

With DSE & MemorySSA, it gives some nice compile-time improvements, due
to the fact that DSE can re-use the PDT from ADCE, if it does not make
any changes:

http://llvm-compile-time-tracker.com/compare.php?from=15fdd6cd7c24c745df1bb419e72ff66fd138aa7e&to=481f494515fc89cb7caea8d862e40f2c910dc994&stat=instructions

Diff Detail

Event Timeline

fhahn created this revision.Sep 8 2020, 12:39 PM
fhahn requested review of this revision.Sep 8 2020, 12:39 PM

Did you investigate a similar change for lib/Passes/PassBuilder.cpp:734->720?

fhahn updated this revision to Diff 290675.Sep 9 2020, 2:18 AM

Did you investigate a similar change for lib/Passes/PassBuilder.cpp:734->720?

I've added the changes for the NewPM as well and updated the test diffs. I'll add numbers for the NewPM once I have them.

sgtm!

It may be good to have this in before the DSE+MSSA flag flip to minimize the regression introduced by that patch (assuming there are no meaningful binary changes or run time regressions for the NPM either).

I forgot to add, if you could include the impact on the NPM for compile-time, it would be great.

fhahn added a comment.Sep 10 2020, 8:15 AM

I forgot to add, if you could include the impact on the NPM for compile-time, it would be great.

Compile-time changes for the NPM: http://195.201.131.214:8000/compare.php?from=adfd684c0c6984a4287bec3711acdc78b0128569&to=ad0eba2e8e73a6aeff18e89d44e3585717c4f018&stat=instructions

-O3                        -0.10%
ReleaseThinLTO    -0.20%
RelaseLTO.            -0.08%

The only binary change is a slight size decrease for consumer-typeset (roughly the same decrease is also present for LPM).

xbolva00 accepted this revision.Sep 19 2020, 12:30 PM
xbolva00 added a subscriber: xbolva00.

lg too

This revision is now accepted and ready to land.Sep 19 2020, 12:30 PM
nikic added a comment.Oct 17 2020, 8:49 AM

Reverse ping, now that MSSA DSE has landed again.

Reverse ping, now that MSSA DSE has landed again.

Just run the numbers again, still looks profitable! I am planning on pushing this tomorrow or the day after

http://llvm-compile-time-tracker.com/compare.php?from=781941183700b52d11b27227e3341385447610fa&to=6797ae92d1a74835efa65f10cdb977a96f921dc4&stat=instructions

fhahn edited the summary of this revision. (Show Details)Oct 21 2020, 2:20 AM
This revision was landed with ongoing or failed builds.Oct 21 2020, 2:31 AM
This revision was automatically updated to reflect the committed changes.