Page MenuHomePhabricator

PR36032 fix assert cause by not computed SCEV PredicatedBackEdgeCount
ClosedPublic

Authored by evstupac on Jan 26 2018, 3:00 PM.

Details

Summary

The change fix an assert fail at ScalarEvolutionExpander.cpp (generateOverflowCheck):

const SCEV *ExitCount =                              
    SE.getPredicatedBackedgeTakenCount(AR->getLoop(), Pred); 
                                                     
assert(ExitCount != SE.getCouldNotCompute() && "Invalid loop count");

(generateOverflowCheck is called only from expandWrapPredicate)

by inserting an early check on SCEV PredicateBackEdgeCount when createAddRecFromPHIWithCastsImpl

Diff Detail

Repository
rL LLVM

Event Timeline

evstupac created this revision.Jan 26 2018, 3:00 PM

Thanks for submitting the fix! Please see the inline comments.

Silviu

lib/Analysis/ScalarEvolution.cpp
4657 ↗(On Diff #131662)

This assumes that L is a sub-loop of the loop being processed by the rewriter.

To make this correct you would also have to add the predicates returned by getPredicatedBackedgeTakenCount to the set of returned predicates.

I think this function might also get called from getPredicatedBackedgeTakenCount and might result in an infinite loop.

I think there is an easier solution (see the comment below).

11500 ↗(On Diff #131662)

I think at least for now the fix can be done here, and should check that all overflow predicates are done on L (the loop currently being processed by the rewriter).

evstupac updated this revision to Diff 133710.Feb 9 2018, 3:58 PM
evstupac added inline comments.
lib/Analysis/ScalarEvolution.cpp
11500 ↗(On Diff #131662)

Moved the check - trying to address your suggestion. Please take a look.

LGTM with one minor nit.

lib/Analysis/ScalarEvolution.cpp
11548 ↗(On Diff #133710)

Not related to this change, but it might be worth not processing Expr if it is invariant in L.

11558 ↗(On Diff #133710)

This should be cast<> instead of dyn_cast since WP->getExpr() is always an AddRecExpr.

evstupac updated this revision to Diff 133946.Feb 12 2018, 3:04 PM
sbaranga accepted this revision.Feb 19 2018, 8:09 AM
This revision is now accepted and ready to land.Feb 19 2018, 8:09 AM
Closed by commit rL326154: Fix PR36032, PR35432 (authored by evstupac, committed by ). · Explain WhyFeb 26 2018, 4:19 PM
This revision was automatically updated to reflect the committed changes.