This is an archive of the discontinued LLVM Phabricator instance.

[IRCE] Recognize loops with unsigned latch conditions
ClosedPublic

Authored by mkazantsev on Jul 12 2017, 5:46 AM.

Diff Detail

Repository
rL LLVM

Event Timeline

mkazantsev added inline comments.Jul 13 2017, 12:21 AM
lib/Transforms/Scalar/InductiveRangeCheckElimination.cpp
1020 ↗(On Diff #106189)

This is actually a bug that also exists for start = SINT_MAX even without this patch. I'm going to fix it separately.

mkazantsev planned changes to this revision.Jul 13 2017, 1:44 AM

Will rebase on top of the fix for the corner case problem.

mkazantsev added inline comments.Jul 13 2017, 1:57 AM
lib/Transforms/Scalar/InductiveRangeCheckElimination.cpp
1020 ↗(On Diff #106189)

Fixed an underlying bug with borders, test_12 have been re-enabled.

anna requested changes to this revision.Jul 28 2017, 10:07 AM

Couple of minor changes. I haven't looked at the tests yet.

Do you mind separating out the ULT/ULE changes from the UGT/UGE just for ease of reviewing? Once the first part is reviewed, the second would be pretty straightforward.

lib/Transforms/Scalar/InductiveRangeCheckElimination.cpp
462 ↗(On Diff #106395)

Pls rename this as isSignedPredicate.

846 ↗(On Diff #106395)

Pls name this as the PredOpCode. I got confused with guards..

978 ↗(On Diff #106395)

This to isSignedPredicate as well. Similarly for all other booleans.

This revision now requires changes to proceed.Jul 28 2017, 10:07 AM
mkazantsev updated this revision to Diff 109068.Aug 1 2017, 3:35 AM
mkazantsev edited edge metadata.
mkazantsev marked 3 inline comments as done.

Reworked tests, made some refactoring, addressed TODOs.

mkazantsev added inline comments.Aug 1 2017, 3:43 AM
lib/Transforms/Scalar/InductiveRangeCheckElimination.cpp
846 ↗(On Diff #106395)

Renamed to BoundPred to avoid confusion. Semantically it is a predicate we need to check to make sure that the indvar lies within bounds.

anna requested changes to this revision.Aug 2 2017, 6:50 AM

Looks almost ready to go. Some comments regarding additional tests.

lib/Transforms/Scalar/InductiveRangeCheckElimination.cpp
1228 ↗(On Diff #109068)

Nit: set to nullptr.

1248 ↗(On Diff #109068)

set to nullptr.

test/Transforms/IRCE/unsigned_comparisons_ugt.ll
9 ↗(On Diff #109068)

Please add some tests or checks for post loops as well. That code's not tested here.

test/Transforms/IRCE/unsigned_comparisons_ult.ll
10 ↗(On Diff #109068)

Please add some tests or checks for post loops as well. That code's not tested here.

This revision now requires changes to proceed.Aug 2 2017, 6:50 AM
mkazantsev updated this revision to Diff 109512.Aug 3 2017, 4:20 AM
mkazantsev edited edge metadata.
mkazantsev marked 4 inline comments as done.

Addressed nits, reworked tests so that now they check the structure of transformed loop, not only the fact that the transformation has happened.

mkazantsev added inline comments.Aug 3 2017, 4:23 AM
test/Transforms/IRCE/eq_ne.ll
18 ↗(On Diff #109512)

Also need to update the tests in the same manner for this file. I will do it later in a separate patch.

anna accepted this revision.Aug 3 2017, 7:15 AM

LGTM.

lib/Transforms/Scalar/InductiveRangeCheckElimination.cpp
1213 ↗(On Diff #109512)

Set to nullptr here as well.

This revision is now accepted and ready to land.Aug 3 2017, 7:15 AM
This revision was automatically updated to reflect the committed changes.
mkazantsev marked an inline comment as done.