Skip to content

[ConstraintAnalysis] Fix asymmetry in provesPair overlap check - #9258

Merged
tlively merged 2 commits into
mainfrom
constraint-analysis-provespair-asymmetry
Oct 13, 2026
Merged

tlively merged 2 commits into
mainfrom
constraint-analysis-provespair-asymmetry

Conversation

@tlively

@tlively tlively commented Oct 13, 2026

Copy link
Copy Markdown
Member

Fixes provesPair to symmetrically check disjointness using proven spans rather than exact spans, since a disjoint superset implies disjoint subsets. This addresses an assertion failure in approximateOr where asymmetric comparison of constraints incorrectly produced an OR-induced contradiction.

Removes unreachable empty() span checks in provesPair since empty spans are only produced by immediate contradictions, which are evaluated to contradictions prior to reaching provesPair.

Fixes `provesPair` to symmetrically check disjointness using proven spans rather than exact spans, since a disjoint superset implies disjoint subsets. This addresses an assertion failure in `approximateOr` where asymmetric comparison of constraints incorrectly produced an OR-induced contradiction.

Removes unreachable `empty()` span checks in `provesPair` since empty spans are only produced by immediate contradictions, which are evaluated to contradictions prior to reaching `provesPair`.
@tlively
tlively requested a review from a team as a code owner October 13, 2026 20:47
@tlively
tlively requested review from kripken and removed request for a team October 13, 2026 20:47
Comment thread src/ir/constraint.cpp
return False;
}
auto bSpans = b.getSpans(type);
auto bProvenSpans = b.getProvenSpans(type);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems confusing to now have aSpans meaning "a's proven spans" while bSpans means "b's exact spans". Perhaps rename one?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done, renamed aSpans to aProvenSpans.

@kripken kripken left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another option might be to change how we assert on when a contradiction arrives, but making it symmetrical seems nicer actually. lgtm % comment on variable names.

@tlively
tlively enabled auto-merge (squash) October 13, 2026 23:17
@tlively
tlively merged commit ca1cf60 into main Oct 13, 2026
16 checks passed
@tlively
tlively deleted the constraint-analysis-provespair-asymmetry branch October 13, 2026 23:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants