This is an archive of the discontinued LLVM Phabricator instance.

[ConstraintElimination] Handle solving-only non-equality predicates
ClosedPublic

Authored by antoniofrighetto on Jun 12 2023, 1:33 AM.

Details

Summary

Simplification of non-equality predicates for solving constraints system is now supported by checking the validity of related inequalities and equalities.

Diff Detail

Event Timeline

Herald added a project: Restricted Project. · View Herald TranscriptJun 12 2023, 1:33 AM
antoniofrighetto requested review of this revision.Jun 12 2023, 1:33 AM
Herald added a project: Restricted Project. · View Herald TranscriptJun 12 2023, 1:33 AM
fhahn added inline comments.Jun 12 2023, 4:04 AM
llvm/lib/Transforms/Scalar/ConstraintElimination.cpp
536

I think this is not correct, as this effectively means we will be adding Op0 <= Op1 to the constraint system as fact for Op0 != Op1 at the moment. To start with, it would be good to only support NE predicates for solving, not when adding facts.

A test case that shows the issue is below. Would be good to have similar variants with UGE/UGT/ULT and signed variants.

define i1 @assume(i64 %a, i64 %b, i64 %c) {
  %1 = icmp ne i64 %a, %b
  tail call void @llvm.assume(i1 %1)
  %r = icmp ule i64 %a, %b
  ret i1 %r
}
llvm/test/Transforms/ConstraintElimination/ne.ll
298

please add the additional test coverage separately. It would also be good to use names for the various values (instead of numbers) so they are easier to modify/play with.

antoniofrighetto added a comment.EditedJun 12 2023, 5:04 AM

Reviewing this, as, indeed, I have been meaning to add this for solving only, not as fact for now, which would need further handling. I believe that simplifying NE to ULE for solving only, as it is being done for EQ, should be OK though. For facts, we should add something close to (Op0 < Op1) ∨ (Op1 > Op0), but I need to think further on how to model this. Perhaps, for now we could return from addFact when if (!R.isValid(*this) || R.IsNe()).

antoniofrighetto retitled this revision from [ConstraintElimination] Handle non-equality predicates to [ConstraintElimination] Handle solving-only non-equality predicates.
antoniofrighetto edited the summary of this revision. (Show Details)
fhahn added inline comments.Jun 12 2023, 8:58 AM
llvm/lib/Transforms/Scalar/ConstraintElimination.cpp
671–673

might be good to expand the comment here, explaining how this handles == and !=. Same below.

llvm/test/Transforms/ConstraintElimination/assumes.ll
626 ↗(On Diff #530510)

better move those to ne.ll and also add variants with ule/uge. Would be good to add the tests separately.

I think for now it should be fine to handle ICMP_NE in solving, support for facts to be added at a later time.

fhahn accepted this revision.Jun 26 2023, 7:53 AM

LGTM, thanks!

llvm/test/Transforms/ConstraintElimination/ne.ll
408

Might be good to add a TODO for signed support.

This revision is now accepted and ready to land.Jun 26 2023, 7:53 AM