HomePhabricator

[reassociate] Fix excessive revisits when processing long chains of…

Authored by dsanders on May 2 2018, 10:59 AM.

Description

[reassociate] Fix excessive revisits when processing long chains of reassociatable instructions.

Summary:
Some of our internal testing detected a major compile time regression which I've
tracked down to:

r278938 - Revert "Reassociate: Reprocess RedoInsts after each inst".

It appears that processing long chains of reassociatable instructions causes
non-linear (potentially exponential) growth in the number of times an
instruction is revisited. For example, the included test revisits instructions
220 times in a 20-instruction test.

It appears that r278938 reversed the order instructions were visited and that
this is preventing scheduled revisits from being cancelled as a result of
visiting the instructions naturally during normal processing. However, simply
reversing the order also harmed the generated code. Upon closer inspection, it
was discovered that revisits occurred in the opposite order to the first pass
(Thanks to escha for spotting that).

This patch makes the revisit order consistent with the first pass which allows
more revisits to be cancelled. This does appear to have a small impact on the
generated code in few cases but it significantly reduces compile-time.

After this patch, our internal test that was most affected by the regression
dropped from ~2 million revisits to ~4k resulting in Reassociate having 0.46%
of the runtime it had before (99.54% improvement).

Here's the summaries reported by lnt for the LLVM test-suite with --benchmarking-only:

metricgeomean before patchgeomean after patchdelta
compile time0.19560.1261-35.54%
execution time0.32400.3237-
code size7365.44597365.6079-

The results have a few wins and losses on compile-time, mostly in the +/- 2.5% range. There was one outlier though:

Performance Regressions - compile_timeΔPreviousCurrent
MultiSource/Benchmarks/ASC_Sequoia/CrystalMk/CrystalMk9.82%2.04732.2483

Reviewers: javed.absar, dberlin

Reviewed By: dberlin

Subscribers: kristof.beyls, llvm-commits

Differential Revision: https://reviews.llvm.org/D45734

llvm-svn: 331381