[ANALYSIS] Fix divisibility estimation for min/max/select ops#7781
Merged
Conversation
Contributor
Author
ptillet
pushed a commit
that referenced
this pull request
Aug 7, 2025
And also simplifies gcd invocation and lifts `kMaxDivisor` into a constant expression
7 tasks
lezcano
pushed a commit
that referenced
this pull request
May 27, 2026
…put contiguity (#10359) Fixes #10067. In `SelectOpAxisInfoVisitor`'s tensor-cond branch, the call to `getDivisibilityFromContiguity` only sees the lhs/rhs contiguities and can overestimate divisibility when `condConstancy` further reduces the output contiguity below either input's contiguity. ### Concrete example - lhs: `[8, 9, 10, 11, 12, 13, 14, 15]` (c=8, d=8) - rhs: `[16, 17, 18, 19, 20, 21, 22, 23]` (c=8, d=16) - Element-wise condition, so `condConstancy = 1` Output contiguity collapses to `gcd(8, 8, 1) = 1` (every position is a leader). But `getDivisibilityFromContiguity` sees `c_lhs == c_rhs == 8` and returns `gcd(8, 16) = 8` — without accounting for `condConstancy`. The output value at position 1 may be 17, which is not divisible by 8. ### Why this didn't blow up On the current pow2 lattice, `gcd == min` on powers of 2. Codegen's `vec_width = min(c, d/e, ...)` is bounded by contiguity, and contiguity is computed correctly. So the overestimated divisibility is never the binding constraint on `vec_width`. This is a latent soundness regression introduced by #7781 — the pre-#7781 code clamped `divisibility` against the just-computed output contiguity, and that was sound independent of the pow2 invariant. ### Fix A conditional GCD with the output contiguity at the SelectOp callsite. The clamp fires only when `condConstancy` (or another shrinking factor) reduces the output contiguity strictly below at least one input's contiguity — i.e. it preserves the existing semantics when `condConstancy` is non-binding. The helper `getDivisibilityFromContiguity` is left unchanged (its other callers — `MaxMinOpAxisInfoVisitor` and `AxisInfo::join` — don't have a `condConstancy`-equivalent, so the same gap doesn't exist there). ### Test Added `select_cond_constancy_clamps_divisibility` in `test/Analysis/test-alignment.mlir`. The test fails before the fix (`divisibility = [8]`) and passes after (`divisibility = [1]`). # New contributor declaration - [x] I am not making a trivial change, such as fixing a typo in a comment. - [x] I have written a PR description following these [rules](https://cbea.ms/git-commit/#why-not-how). - [x] I have run `pre-commit run --from-ref origin/main --to-ref HEAD`. - Select one of the following. - [x] I have added tests. - `/test` for `lit` tests - `/unittest` for C++ tests - `/python/test` for end-to-end tests - [ ] This PR does not need a test because `FILL THIS IN`. - Select one of the following. - [ ] I have not added any `lit` tests. - [x] The `lit` tests I have added follow these [best practices](https://mlir.llvm.org/getting_started/TestingGuide/#filecheck-best-practices), including the "tests should be minimal" section. (Usually running Python code and using the instructions it generates is not minimal.) --------- Co-authored-by: Keren Zhou <kerenzhou@openai.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
And also simplifies gcd invocation and lifts
kMaxDivisorinto a constant expression