Page MenuHomePhabricator

Partially fix PR20058: reduce compile time for loop unrolling with very high count
Needs ReviewPublic

Authored by meheff on Jun 24 2014, 11:41 PM.

Details

Reviewers
eliben
Summary

This patch removes redundant calls to ScalarEvolution::forgetLoop when loop unrolling. For very high loop count loops this cuts the compile time (such as the one in the PR) by about a factor of two.

Diff Detail

Event Timeline

meheff updated this revision to Diff 10818.Jun 24 2014, 11:41 PM
meheff retitled this revision from to Partially fix PR20058: reduce compile time for loop unrolling with very high count.
meheff updated this object.
meheff edited the test plan for this revision. (Show Details)
meheff added a reviewer: eliben.
meheff added a subscriber: Unknown Object (MLST).
hfinkel added inline comments.
lib/Transforms/Utils/LoopUnroll.cpp
438–439

How do you know that LI->getLoopFor(Dest) returns the same Loop for all Dest in this loop?

meheff updated this revision to Diff 10838.Jun 25 2014, 9:28 AM
meheff added inline comments.
lib/Transforms/Utils/LoopUnroll.cpp
438–439

Good point. Changed to using a pointer set to keep track of forgotten loops.

LGTM. Thanks!

Also, can you add an explicit note in ScalarEvolution.h above the forgetLoop declaration warning that the call is potentially expensive.

meheff updated this revision to Diff 11282.Jul 10 2014, 11:51 AM

On Wed, Jun 25, 2014 at 9:34 AM, Eli Bendersky <eliben@google.com> wrote:

Is this part of the code well covered by tests? Maybe some more targeted
tests can be crafted.

There are a few tests which require forgetting more than one loop (eg test/Transforms/LoopUnroll/pr11361.ll). One case where this can occur is if an inner loop is fully unrolled. In this case both the inner loop and the containing loop are forgotten.

On Mon, Jun 30, 2014 at 3:29 PM, hfinkel@anl.gov <hfinkel@anl.gov> wrote:

Also, can you add an explicit note in ScalarEvolution.h above the forgetLoop declaration warning that the call is potentially expensive.

Done.

commited as r212782.