Page MenuHomePhabricator

[MLIR] Simplex::pivot: also update the redundant rows when pivoting
ClosedPublic

Authored by arjunp on Thu, Nov 25, 1:39 PM.

Details

Summary

Previously, the pivot function would only update the non-redundant rows when
pivoting. This is incorrect because in some cases, when rolling back past a
detectRedundant call, the basis being used could be different from that which
was used at the time of returning from the detectRedundant call. Therefore,
it is important to update the redundant rows as well during pivots. This could
also be triggered by pivots that occur when testing successive constraints for
being redundant in detectRedundant after some initial constraints are marked redundant.

Diff Detail

Event Timeline

arjunp created this revision.Thu, Nov 25, 1:39 PM
arjunp requested review of this revision.Thu, Nov 25, 1:39 PM
grosser added inline comments.Thu, Nov 25, 6:48 PM
mlir/unittests/Analysis/Presburger/SimplexTest.cpp
14

This header is unused except in the commented code.

398

I guess you do not want to add this commented code?

arjunp updated this revision to Diff 389976.Fri, Nov 26, 3:00 AM

Remove commented out test and header. Improve test documentation.

arjunp marked 2 inline comments as done.Fri, Nov 26, 3:01 AM
arjunp added inline comments.
mlir/unittests/Analysis/Presburger/SimplexTest.cpp
398

Right. Thanks for pointing it out.

There is a typo in the commit message: deteectRedundant

arjunp edited the summary of this revision. (Show Details)Fri, Nov 26, 3:14 AM
arjunp marked an inline comment as done.

Fixed the commit message.

Groverkss accepted this revision.Fri, Nov 26, 5:50 AM

LGTM! Thank you for the fix.

This revision is now accepted and ready to land.Fri, Nov 26, 5:50 AM
arjunp updated this revision to Diff 390050.Fri, Nov 26, 7:11 AM

Try to rebase. I don't understand why the windows build fails on something in flang, and consistently as well.

arjunp updated this revision to Diff 390060.Fri, Nov 26, 7:43 AM

I accidentally replaced this with a different patch, fixed now.

Maybe the windows build is just broken. If this is clearly unrelated, I would not hold this patch back.

It seems to have worked now.