This is an archive of the discontinued LLVM Phabricator instance.

[ConstraintElimination] Add tests to check for transfering facts from sgt to ugt.
ClosedPublic

Authored by zjaffal on Apr 14 2023, 3:32 AM.

Details

Summary

If we have a >s b and we know that b >s 0 then we also can infer that a >u b.

Diff Detail

Event Timeline

zjaffal created this revision.Apr 14 2023, 3:32 AM
Herald added a project: Restricted Project. · View Herald TranscriptApr 14 2023, 3:32 AM
Herald added a subscriber: StephenFan. · View Herald Transcript
zjaffal requested review of this revision.Apr 14 2023, 3:32 AM
Herald added a project: Restricted Project. · View Herald TranscriptApr 14 2023, 3:32 AM
zjaffal updated this revision to Diff 513523.Apr 14 2023, 3:43 AM

update test cases to use uge instead of ugt

zjaffal retitled this revision from [ConstraintElimination] Add tests to check for transfering facts from sgt to ugt. to [ConstraintElimination] Add tests to check for transfering facts from sgt to uge..Apr 14 2023, 3:43 AM
zjaffal edited the summary of this revision. (Show Details)
zjaffal updated this revision to Diff 513533.Apr 14 2023, 4:10 AM

change back to ugt

zjaffal retitled this revision from [ConstraintElimination] Add tests to check for transfering facts from sgt to uge. to [ConstraintElimination] Add tests to check for transfering facts from sgt to ugt..Apr 14 2023, 4:10 AM
zjaffal edited the summary of this revision. (Show Details)
fhahn added a comment.Apr 14 2023, 5:40 AM

Thanks for adding the extra tests. A few suggestions about additional coverage inline

llvm/test/Transforms/ConstraintElimination/transfer-signed-facts-to-unsigned.ll
599

Could you add a variant where we also check icmp ugt i8 %a, 1 (negative test)

626

please also add a variant of the test where we have no info about %b's range (i.e. no check for %b).

642

please also add a test with a different negative value.

645

please don't use %t.x prefix for values that are not expected to be simplified; better just use %c. (here and below)

zjaffal updated this revision to Diff 513585.Apr 14 2023, 7:22 AM

add more tests

fhahn accepted this revision.Apr 14 2023, 7:34 AM

LGTM, thanks!

This revision is now accepted and ready to land.Apr 14 2023, 7:34 AM
This revision was landed with ongoing or failed builds.Apr 14 2023, 7:44 AM
This revision was automatically updated to reflect the committed changes.