This is an archive of the discontinued LLVM Phabricator instance.

[NFC][SCEV] Give hints on why we forget loops
AbandonedPublic

Authored by mkazantsev on Feb 22 2022, 3:08 AM.

Details

Summary

Following discussion in D120303, introduce API that allows us to
notify SCEV that some loop will be broken or entirely deleted with
all its subloops, and therefore there is no need to store cached data
for them. The conservative fallback is "loop may stay, keep dependencies
that should stay alive".

Diff Detail

Unit TestsFailed

Event Timeline

mkazantsev created this revision.Feb 22 2022, 3:08 AM
mkazantsev requested review of this revision.Feb 22 2022, 3:08 AM
Herald added a project: Restricted Project. · View Herald TranscriptFeb 22 2022, 3:08 AM
mkazantsev abandoned this revision.Mar 4 2022, 12:20 AM
Herald added a project: Restricted Project. · View Herald TranscriptMar 4 2022, 12:20 AM
reames added a comment.Mar 7 2022, 8:57 AM

I'm having a hard time wrapping my head around what this patch does and is expected to do.

If I'm reading the prior review correct, clearing the LoopUsers can result in functional bugs. Given that, this "hint" isn't really a hint. You must get it right, or you may have a functional bug. But this patch seems to be adding more cases where we clear LoopUsers? Is this a functional fix?

If not, what is the compile time impact of leaving LoopUsers in place when we could clear it? Is it significant? If not, introducing complexity here does not seem worthwhile.

As an aside, I'd naively expect the loop association to be cleared when the AddRec is. Is there a reason we can't identify the loop dependent SCEVS, forgetValue them, and let forgetValue handle the actual map manipulation? Seems conceptually cleaner and less error prone. (Unless I'm missing something - which is very possible.)