This paves the way for integer-exact projection, and for supporting
non-division locals in subtraction, complement, and equality checks.
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
mlir/lib/Analysis/Presburger/IntegerRelation.cpp | ||
---|---|---|
167–175 | I think a better way to write this loop would be to write it in reverse. Swapping each non-div with the offset + numDivisions and incrementing numDivisions after each iteration. | |
167–175 | Also, please write some comments explaining what this does. I find this somewhat non-trivial to understand. | |
1172–1174 | Not for this patch, but I do not like that we compute the division representation each time we want it. This computation is very expensive. We should instead be storing these divisions separately with each set. |
mlir/lib/Analysis/Presburger/IntegerRelation.cpp | ||
---|---|---|
167–175 | numDivisions does not get incremented on every iteration in that implementation. i is only incremented in some cases as well, same as here. Why are you saying that way is better? |
mlir/lib/Analysis/Presburger/IntegerRelation.cpp | ||
---|---|---|
167–175 | Sorry for being unclear. I was just suggesting that I think it is more natural to write this loop in reverse. I find that loops working with divisions are easier to write when written in reverse. But since you are not using the division representation, this should be either way. |
Fix bug where the resulting set had a space incompatible with the space of the original set.
mlir/include/mlir/Analysis/Presburger/IntegerRelation.h | ||
---|---|---|
101–104 | This is very confusing and I cannot understand what this does. Can you please write it more clearly? | |
747–749 | I think writing about union of convex disjuncts is enough. | |
mlir/include/mlir/Analysis/Presburger/PresburgerRelation.h | ||
60 | I would rather call them "extra" ids. | |
63 | ||
mlir/unittests/Analysis/Presburger/PresburgerSetTest.cpp | ||
764 | I think you should put a newline after this. |
This is very confusing and I cannot understand what this does. Can you please write it more clearly?