This is an archive of the discontinued LLVM Phabricator instance.

[ConstraintElim] Handle trivial (ICMP_ULE, 0, B) in doesHold.
ClosedPublic

Authored by fhahn on Aug 24 2023, 1:57 PM.

Details

Summary

D152730 may add trivial pre-conditions of the form (ICMP_ULE, 0, B),
which won't be handled automatically by the constraint system, because
we don't add Var >= 0 for all variables in the unsigned system.

Handling the trivial condition explicitly here avoids having the
increase the number of rows in the system per variable.

https://alive2.llvm.org/ce/z/QC92ur

Depends on D152730.

Fixes #63125.

Diff Detail

Event Timeline

fhahn created this revision.Aug 24 2023, 1:57 PM
Herald added a project: Restricted Project. · View Herald TranscriptAug 24 2023, 1:57 PM
fhahn requested review of this revision.Aug 24 2023, 1:57 PM
Herald added a project: Restricted Project. · View Herald TranscriptAug 24 2023, 1:57 PM
nikic added inline comments.Aug 25 2023, 3:26 AM
llvm/lib/Transforms/Scalar/ConstraintElimination.cpp
759

Is it guaranteed that zero will be on the left?

fhahn updated this revision to Diff 554346.Aug 29 2023, 8:05 AM

Update to also handle (ICMP_UGE, B, 0).

fhahn marked an inline comment as done.Aug 29 2023, 8:08 AM
fhahn added inline comments.
llvm/lib/Transforms/Scalar/ConstraintElimination.cpp
759

For the preconditions, yes, but in general no. I extended the logic to also handle UGE B, 0. This simplifies a bunch of checks in the tests, but won't change much with the current pipeline position.

fhahn marked an inline comment as done.Sep 22 2023, 7:33 AM

ping :)

fhahn updated this revision to Diff 557396.Sep 27 2023, 3:12 AM

Rebase after landing D152730, ping :)

nikic accepted this revision.Sep 27 2023, 3:40 AM

LGTM

This revision is now accepted and ready to land.Sep 27 2023, 3:40 AM
This revision was landed with ongoing or failed builds.Sep 27 2023, 4:12 AM
This revision was automatically updated to reflect the committed changes.