This is an archive of the discontinued LLVM Phabricator instance.

[ConstraintElimination] Allow adding pre-conditions for constraints.
ClosedPublic

Authored by fhahn on Feb 2 2022, 7:58 AM.

Details

Summary

With this patch pre-conditions can be added to a list of constraints.
Constraints with pre-conditions can only be used if all pre-conditions
are satisfied when the constraint is used.

The pre-conditions at the moment are specified as a list of
(Predicate, Value *,Value *) tuples. This allow easily checking them
like any other condition, using the existing infrastructure.

This then is used to limit GEP decomposition to cases where we can
prove that offsets are signed positive.

This fixes a couple of incorrect transforms where GEP offsets where
assumed to be signed positive, but they were not.

Note that this effectively disables GEP decomposition, as there's no
support for reasoning about signed predicates. I will post a patch to do
so soon.

Fixes PR49624.

Diff Detail

Event Timeline

fhahn created this revision.Feb 2 2022, 7:58 AM
fhahn requested review of this revision.Feb 2 2022, 7:58 AM
Herald added a project: Restricted Project. · View Herald TranscriptFeb 2 2022, 7:58 AM
reames accepted this revision.Feb 3 2022, 8:09 AM

LGTM

Code looks reasonable, don't have a ton of context so I'm assuming the existing code does what it says. If you want a more rigorous review, ask and I'll take a more skeptical pass through.

llvm/lib/Transforms/Scalar/ConstraintElimination.cpp
82

I'll note we can do smarter merges here in some cases, but we can defer fanciness until warranted.

This revision is now accepted and ready to land.Feb 3 2022, 8:09 AM
fhahn marked an inline comment as done.Feb 4 2022, 3:18 AM

LGTM

Code looks reasonable, don't have a ton of context so I'm assuming the existing code does what it says. If you want a more rigorous review, ask and I'll take a more skeptical pass through.

Thanks for taking a look, the general/high-level sanity check is very much appreciated!

llvm/lib/Transforms/Scalar/ConstraintElimination.cpp
82

Good point! I'll add a TODO.

This revision was landed with ongoing or failed builds.Feb 4 2022, 3:45 AM
This revision was automatically updated to reflect the committed changes.
fhahn marked an inline comment as done.