ConstraintAnalysis: Sort constraints internally#8900
Conversation
| push_back(c); | ||
| sort(); |
There was a problem hiding this comment.
It would be slightly more efficient to use std::upper_bound and inplace_vector::insert here to maintain the invariant that the vector is sorted.
| if (op != other.op) { | ||
| return op < other.op; | ||
| } |
There was a problem hiding this comment.
It may be worth taking on extra complexity to have more precise comparisons ordered lower to prioritize keeping them. For example, eq lt gt le ge ne might be a good order.
There was a problem hiding this comment.
Good idea, added a TODO.
|
|
||
| bool Literal::operator<(const Literal& other) const { | ||
| if (type != other.type) { | ||
| return type.getID() < other.type.getID(); |
There was a problem hiding this comment.
This is not going to be deterministic, which may or may not be a problem. We could alternatively do a breadth-first comparison on the structure of the value. Probably not worth implementing now, but we can add a TODO comment about it.
There was a problem hiding this comment.
Yeah, this is not deterministic between runs, the FuncData too. I'll add a comment.
| return name == other.name && self == other.self; | ||
| } | ||
| bool operator<(const FuncData& other) const { | ||
| return std::tie(name, self) < std::tie(other.name, other.self); |
| ;; Regression test for an infinite loop: the specific cfg here + the | ||
| ;; constraints lead to a situation where, if we were not careful, we would | ||
| ;; think we have an infinite stream of updates in flow(). Specifically, we | ||
| ;; end up updating a location to a combination of two constraints {A, B} and | ||
| ;; then end up finding {B, A} in the next cycle, and then alternate those | ||
| ;; two forever. This is fixed by sorting the constraints. | ||
| ;; | ||
| ;; (There is nothing to optimize here, we just should not hang or error.) |
There was a problem hiding this comment.
A lattice wouldn't have had this problem 🥲
2 participants
Sorting them in each set of ANDed constraints is not only nice for
debugging but makes comparison simpler: they compare equal
regardless of order.
This adds some boilerplate
operator<in necessary places.