Open
Conversation
ftynse
reviewed
Mar 16, 2026
Comment on lines
+70
to
+71
| # Module-level context reused across calls to benefit from hash-consing. | ||
| _ixs_ctx = ixsimpl.Context() |
Contributor
Author
There was a problem hiding this comment.
That's an interesting question, the C context itself is not thread safe, but it would be safe if called it under GIL. Otherwise, we can try to create thread-local contexts.
Contributor
There was a problem hiding this comment.
GIL is optional since Python 3.13 https://peps.python.org/pep-0703/. Since the context is essentially a cache, making it thread-local sounds reasonable.
| result = ixs_simplify(expr) | ||
| if result is not expr: | ||
| return result | ||
| # Fallback: ixs_simplify returned expr unchanged (conversion error). |
Contributor
There was a problem hiding this comment.
You may want to differentiate the situation where there was a conversion error from an expression that wasn't simplified without errors.
Hardcode84
added a commit
to Hardcode84/wave
that referenced
this pull request
Mar 19, 2026
Address PR iree-org#1130 review feedback: - Use threading.local() for ixsimpl context instead of a module-level global, since the C context is not thread-safe and GIL is optional since Python 3.13. - Extract _ixs_simplify_core that propagates conversion exceptions, so simplify() only falls back to the sympy path on actual conversion failures, not when ixsimpl processed the expression successfully but could not simplify it further. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Ivan Butygin <ivan.butygin@gmail.com>
Contributor
Author
|
(undrafting so tests can actually run) |
f628523 to
7a78aa0
Compare
Contributor
Author
|
extracted waveasm fix #1156 |
Hardcode84
added a commit
to Hardcode84/wave
that referenced
this pull request
Mar 20, 2026
Address PR iree-org#1130 review feedback: - Use threading.local() for ixsimpl context instead of a module-level global, since the C context is not thread-safe and GIL is optional since Python 3.13. - Extract _ixs_simplify_core that propagates conversion exceptions, so simplify() only falls back to the sympy path on actual conversion failures, not when ixsimpl processed the expression successfully but could not simplify it further. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Ivan Butygin <ivan.butygin@gmail.com>
Hardcode84
added a commit
to Hardcode84/wave
that referenced
this pull request
Mar 24, 2026
Address PR iree-org#1130 review feedback: - Use threading.local() for ixsimpl context instead of a module-level global, since the C context is not thread-safe and GIL is optional since Python 3.13. - Extract _ixs_simplify_core that propagates conversion exceptions, so simplify() only falls back to the sympy path on actual conversion failures, not when ixsimpl processed the expression successfully but could not simplify it further. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Ivan Butygin <ivan.butygin@gmail.com>
Contributor
Author
|
@harsh-nod @ftynse added an initial doc with gaps analysis |
Hardcode84
added a commit
to Hardcode84/wave
that referenced
this pull request
Mar 26, 2026
Address PR iree-org#1130 review feedback: - Use threading.local() for ixsimpl context instead of a module-level global, since the C context is not thread-safe and GIL is optional since Python 3.13. - Extract _ixs_simplify_core that propagates conversion exceptions, so simplify() only falls back to the sympy path on actual conversion failures, not when ixsimpl processed the expression successfully but could not simplify it further. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Ivan Butygin <ivan.butygin@gmail.com>
Hardcode84
added a commit
to Hardcode84/wave
that referenced
this pull request
Mar 27, 2026
Address PR iree-org#1130 review feedback: - Use threading.local() for ixsimpl context instead of a module-level global, since the C context is not thread-safe and GIL is optional since Python 3.13. - Extract _ixs_simplify_core that propagates conversion exceptions, so simplify() only falls back to the sympy path on actual conversion failures, not when ixsimpl processed the expression successfully but could not simplify it further. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Ivan Butygin <ivan.butygin@gmail.com>
Hardcode84
added a commit
to Hardcode84/wave
that referenced
this pull request
Mar 30, 2026
Address PR iree-org#1130 review feedback: - Use threading.local() for ixsimpl context instead of a module-level global, since the C context is not thread-safe and GIL is optional since Python 3.13. - Extract _ixs_simplify_core that propagates conversion exceptions, so simplify() only falls back to the sympy path on actual conversion failures, not when ixsimpl processed the expression successfully but could not simplify it further. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Ivan Butygin <ivan.butygin@gmail.com>
Hardcode84
added a commit
to Hardcode84/wave
that referenced
this pull request
Apr 7, 2026
Address PR iree-org#1130 review feedback: - Use threading.local() for ixsimpl context instead of a module-level global, since the C context is not thread-safe and GIL is optional since Python 3.13. - Extract _ixs_simplify_core that propagates conversion exceptions, so simplify() only falls back to the sympy path on actual conversion failures, not when ixsimpl processed the expression successfully but could not simplify it further. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Ivan Butygin <ivan.butygin@gmail.com>
Hardcode84
added a commit
to Hardcode84/wave
that referenced
this pull request
Apr 8, 2026
Address PR iree-org#1130 review feedback: - Use threading.local() for ixsimpl context instead of a module-level global, since the C context is not thread-safe and GIL is optional since Python 3.13. - Extract _ixs_simplify_core that propagates conversion exceptions, so simplify() only falls back to the sympy path on actual conversion failures, not when ixsimpl processed the expression successfully but could not simplify it further. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Ivan Butygin <ivan.butygin@gmail.com>
Hardcode84
added a commit
to Hardcode84/wave
that referenced
this pull request
Apr 9, 2026
Address PR iree-org#1130 review feedback: - Use threading.local() for ixsimpl context instead of a module-level global, since the C context is not thread-safe and GIL is optional since Python 3.13. - Extract _ixs_simplify_core that propagates conversion exceptions, so simplify() only falls back to the sympy path on actual conversion failures, not when ixsimpl processed the expression successfully but could not simplify it further. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Ivan Butygin <ivan.butygin@gmail.com>
Introduce ixs_simplify() in symbol_utils.py: converts a sympy expression to ixsimpl, simplifies with assumptions derived from sympy symbol properties, and converts back. Uses a module-level ixsimpl.Context for hash-consing reuse across calls. Accepts optional extra_assumptions for future divisibility plumbing. Remove the debug print statements and throwaway ixsimpl hack from the custom simplify() wrapper. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Ivan Butygin <ivan.butygin@gmail.com>
Switch 4 equivalence-check call sites from sympy.simplify to ixs_simplify roundtrip: _check_expr_equivalent (2 sites) and _sympy_equiv (2 sites). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Ivan Butygin <ivan.butygin@gmail.com>
Switch 3 call sites in _check_index_difference_is_zero() from sympy.simplify to ixs_simplify roundtrip. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Ivan Butygin <ivan.butygin@gmail.com>
Switch 2 equivalence checks in _is_scale_preshuffle_mapping() from sympy.simplify to ixs_simplify roundtrip. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Ivan Butygin <ivan.butygin@gmail.com>
Switch 3 call sites in ensure_symbols_positive() from sympy.simplify to ixs_simplify roundtrip. The remapped positive/nonnegative symbol assumptions are picked up automatically by extract_assumptions. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Ivan Butygin <ivan.butygin@gmail.com>
…rite.py Switch the standalone sympy.simplify call on the multicast mask expression to ixs_simplify roundtrip. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Ivan Butygin <ivan.butygin@gmail.com>
Switch the sympy.simplify call in convert_sympy_expr_to_affine() to ixs_simplify roundtrip. The positive symbol assumptions from the symbol_mapping substitution are captured automatically. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Ivan Butygin <ivan.butygin@gmail.com>
Switch the Piecewise mask simplification in compute_multicast_mask() to ixs_simplify roundtrip. This was a known slow path with sympy.simplify on complex Piecewise expressions. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Ivan Butygin <ivan.butygin@gmail.com>
Delegate simplify() to ixs_simplify as the primary simplification engine. Falls back to the old sympy expand + bounds + cancel loop only when ixs_simplify returns the expression unchanged (indicating a conversion error). Remove the _simplify_cache dict since ixsimpl handles structural deduplication via hash-consing. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Ivan Butygin <ivan.butygin@gmail.com>
- wave_utils_test.py: Update Mod simplification assertion to match ixsimpl's canonical form (Mod(4*a, 16) vs 4*Mod(a, 4)). - mma.py LIT test: Relax cluster mask CHECK lines since ixsimpl flattens Piecewise into individual selects rather than a cascaded chain. Both forms are algebraically equivalent. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Ivan Butygin <ivan.butygin@gmail.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Ivan Butygin <ivan.butygin@gmail.com>
Address PR iree-org#1130 review feedback: - Use threading.local() for ixsimpl context instead of a module-level global, since the C context is not thread-safe and GIL is optional since Python 3.13. - Extract _ixs_simplify_core that propagates conversion exceptions, so simplify() only falls back to the sympy path on actual conversion failures, not when ixsimpl processed the expression successfully but could not simplify it further. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Ivan Butygin <ivan.butygin@gmail.com>
Signed-off-by: Ivan Butygin <ivan.butygin@gmail.com>
Signed-off-by: Ivan Butygin <ivan.butygin@gmail.com>
When Pow(X, -1) is codegen'd and X contains a division (producing a _Rational on the stack), the base must be resolved to an MLIR value before wrapping in a new _Rational. Otherwise a nested _Rational propagates to floor/floordiv_expr which expects flat numerator and denominator values. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Ivan Butygin <ivan.butygin@gmail.com>
Documents the incremental strategy for transitioning from sympy to ixsimpl as the primary expression IR, with API gap analysis and a 6-phase plan where phases 1/2/4 can run in parallel. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Ivan Butygin <ivan.butygin@gmail.com>
ixsimpl now covers every sympy feature used in Wave: lambdify() and Expr.eval() replace sympy.lambdify, Expr.has() is native, abs_() is provided, and ctx.check() replaces the sympy.solve entailment check. No hard API gaps remain; end state is full sympy removal. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Ivan Butygin <ivan.butygin@gmail.com>
Signed-off-by: Ivan Butygin <ivan.butygin@gmail.com>
Signed-off-by: Ivan Butygin <ivan.butygin@gmail.com>
ixsimpl is now vendored as a subfolder, so the pip git dependency is no longer needed. Updated affine map CHECK lines in gather_to_shared and gemm lit tests to match the new canonical form where `mod` is expanded to `x - (x floordiv N) * N`. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Ivan Butygin <ivan.butygin@gmail.com>
sympy.simplify cannot reduce floor/mod identities like -16*t + 16*(t%16) + 256*floor(t/16). Replace with ixsimpl-backed simplify from symbol_utils which handles these natively. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Ivan Butygin <ivan.butygin@gmail.com>
Document the current ixsimpl regression where floor(Mod(4*x + 3, 16) / 16) fails to fold to zero so the gap stays visible without breaking the suite. Signed-off-by: Ivan Butygin <ivan.butygin@gmail.com>
Bring third_party/ixsimpl forward to upstream commit 0032f44 so scaled Mod bounds fold floor(Mod(4*x + 3, 16) / 16) to zero. Drop the temporary xfail now that the vendored simplifier handles the regression. Signed-off-by: Ivan Butygin <ivan.butygin@gmail.com>
The newer vendored ixsimpl seeds the persistent loop directly from %block_id_x, so drop the stale index_cast roundtrip expectation from the lit test. Signed-off-by: Ivan Butygin <ivan.butygin@gmail.com>
Signed-off-by: Ivan Butygin <ivan.butygin@gmail.com>
This reverts commit 7138f64. Signed-off-by: Ivan Butygin <ivan.butygin@gmail.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
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.
https://github.com/Hardcode84/4/tree/master