This is an archive of the discontinued LLVM Phabricator instance.

[SCEV] A different fix for PR33494
ClosedPublic

Authored by sanjoy on Oct 26 2017, 11:02 PM.

Details

Summary

I don't think rL309080 is the right fix for PR33494 -- caching ExitLimit only
hides the problem[0]. The real issue is that because of how we forget SCEV
expressions ScalarEvolution::getBackedgeTakenInfo, in the test case for PR33494
computing the backedge for any loop invalidates the trip count for every other
loop. This effectively makes the SCEV cache useless.

I've instead made the SCEV expression invalidation in
ScalarEvolution::getBackedgeTakenInfo less aggressive to fix this issue.

[0]: One way to think about this is that rL309080 essentially augmented the
backedge-taken-count cache with another equivalent exit-limit cache. The bug
went away because we were explicitly not clearing the exit-limit cache in
getBackedgeTakenInfo. But instead of doing all of that, we can just avoid
clearing the backedge-taken-count cache.

Diff Detail

Repository
rL LLVM

Event Timeline

sanjoy created this revision.Oct 26 2017, 11:02 PM
mkazantsev added inline comments.Oct 31 2017, 12:09 PM
lib/Analysis/ScalarEvolution.cpp
6384 ↗(On Diff #120540)

Do we want to check whether the instruction is already enqueued before we push it?

sanjoy added inline comments.Nov 24 2017, 1:15 PM
lib/Analysis/ScalarEvolution.cpp
6384 ↗(On Diff #120540)

That's what the existing behavior is -- I'll prefer fixing it in a separate NFC change if you don't mind.

mkazantsev accepted this revision.Nov 27 2017, 12:00 AM

Ok. LGTM.

This revision is now accepted and ready to land.Nov 27 2017, 12:00 AM
This revision was automatically updated to reflect the committed changes.