This is an archive of the discontinued LLVM Phabricator instance.

[LoopUnswitch] Fix SCEV invalidation in unswitching
ClosedPublic

Authored by mkazantsev on Apr 25 2018, 12:26 AM.

Details

Summary

Loop unswitching makes substantial changes to a loop that can also affect cached
SCEV info in its outer loops as well, but it only cares to invalidate SCEV cache for the
innermost loop in case of full unswitching and does not invalidate anything at all in
case of trivial unswitching. As result, we may end up with incorrect data in cache.

Diff Detail

Event Timeline

mkazantsev created this revision.Apr 25 2018, 12:26 AM
mkazantsev updated this revision to Diff 144978.May 2 2018, 8:54 PM
mkazantsev retitled this revision from [LoopUnswitch] Fix potentially incorrect SCEV invalidation in unswitching to [LoopUnswitch] Fix SCEV invalidation in unswitching.
mkazantsev edited the summary of this revision. (Show Details)
mkazantsev added a reviewer: uabelho.

This one appeared to be a real bug, at least we have a test that fails with assert for trivial unswitching. Thanks @uabelho for providing the test case.

Thanks Max! This solves the problem I saw!

Not sure if I'm allowed to "LGTM" this but the change makes sense to me.

Actually particularly with case of unswitching, I grow hesitant about one of our asserts that fails here. Maybe it is over-conservative. See comment https://reviews.llvm.org/D44676#1086144

I will check if its removal actually helps.

Never mind, in this case the latch of outer loop changes, so we must clear cache.

mzolotukhin accepted this revision.May 21 2018, 1:57 PM
mzolotukhin added a subscriber: mzolotukhin.

Looks correct to me.

This revision is now accepted and ready to land.May 21 2018, 1:57 PM
This revision was automatically updated to reflect the committed changes.