Page MenuHomePhabricator

Prepare for multi-exit LFTR [NFC]
ClosedPublic

Authored by reames on Jun 4 2019, 2:25 PM.

Details

Summary

This change does the plumbing to wire an ExitingBB parameter through the LFTR implementation, and reorganizes the code to work in terms of a set of individual loop exits. Most of it is fairly obvious, but there's one key complexity which makes it worthy of consideration. (The actual multi-exit LFTR patch is in D62625 for context.)

Specifically, it turns out the existing code uses the backedge taken count from before a IV is widened. This means that we can end up with a different a narrower BE count for the loop than requerying after widening.

For the nestedIV example from elim-extend, we end up with the following BE counts:
BEFORE: (-2 + (-1 * %innercount) + %limit)
AFTER: (-1 + (sext i32 (-1 + %limit) to i64) + (-1 * (sext i32 %innercount to i64))<nsw>)

This is the only test in tree which seems sensitive to this difference. The actual result of using the wider BETC on this example is that we actually produce slightly better code. :)

For the moment, the code is actually NFC. I'd like to land this as is, and then adjust the BETC used in a separate patch. (In particular, I want to investigate *why* they're different. They should be equivalent.) I'm open to being convinced that either a) I should investigate that first, or 2) I should just include the resultng test change in this commit and not worry about it. Thoughts?

Diff Detail

Event Timeline

reames created this revision.Jun 4 2019, 2:25 PM
Herald added a project: Restricted Project. · View Herald TranscriptJun 4 2019, 2:25 PM
reames edited the summary of this revision. (Show Details)Jun 4 2019, 2:26 PM
sanjoy accepted this revision.Jun 6 2019, 9:22 PM

I should just include the resultng test change in this commit and not worry about it.

I would say don't worry about it. This should be rare, but not unheard of. E.g. there are extreme cases where SCEV will be able to constant for A-B but not B-A (where A and B are complex expressions).

lib/Transforms/Scalar/IndVarSimplify.cpp
2201

Generalize

2203

It would probably be more obvious if you did

bool HasOnlyOneExitingBlock = L->getExitingBlock() != nullptr;
if (HasOnlyOneExitingBlock) {
  ...
}

or something like that.

But isn't this called only when the loop has one exiting block (so the check will always return true)?

2650

I think this comment can be clearer about how it ties to the code. Are you saying asking SCEV to recompute getBackedgeTakenCount here can produce different results so you take care to "re-use" BackedgeTakenCount?

This revision is now accepted and ready to land.Jun 6 2019, 9:22 PM
This revision was automatically updated to reflect the committed changes.