[REFACTOR][ARITH] Phase out arith/scalable_expression; arith no longer proves over scalable vectors#19638
Merged
Merged
Conversation
…rite_simplify, tirx/ir/expr) ExtractVscaleFactor is a target-free pattern recognizer (matches `multiplier * vscale()`) that does not belong in the arith layer. Add an anonymous-namespace ExtractVscaleFactor in src/arith/rewrite_simplify.cc (verbatim body using the existing pattern_match machinery) covering the 4 simplify call sites. Add an equivalent file-local helper in src/tirx/ir/expr.cc (raw AST walk, no pattern_match dependency) covering the 2 Ramp/Broadcast constructor call sites. Both files no longer include scalable_expression.h for this function. The function is small enough that two-file duplication is cleaner than a shared cross-subsystem header.
… at call sites IsVScaleCall (3-line predicate on tirx::builtin::vscale()) and ContainsVscaleCall (1-line wrapper over CheckContains::ExprContains) are tirx-level recognizers, not arith-level. TargetHasVLA belongs in the one transform that actually needs it. Each consumer file now declares a file-local anonymous-namespace helper containing the same logic, and drops the arith::scalable_expression.h include for these symbols: - src/arith/rewrite_simplify.cc (ContainsVscaleCall x4, IsVScaleCall used internally) - src/tirx/op/op.cc (IsVScaleCall x1) - src/tirx/transform/vectorize_loop.cc (IsVScaleCall x1, TargetHasVLA x2) - src/s_tir/schedule/ir_comparator.cc (ContainsVscaleCall x1) vectorize_loop.cc also receives the TargetHasVLA file-local helper here, since it is the only non-proof consumer and is already having its include rewritten in this commit.
…r/const_int_bound The arith layer no longer attempts to prove anything about expressions containing vscale() calls. This is intentional: reasoning about scalable-vector lengths at compile time requires Target::Current(), a runtime-mutable global, which does not belong in the arith analysis layer. Changes: - src/arith/analyzer.cc: delete the substitution-proof block (lines 234-250) that iterated over known vscale values via TargetHasVLA + GetVScaleValues + CanProveVscaleExpressionFromKnownValues. vscale-bearing expressions now fall through to `return false` in CanProve. Drop scalable_expression.h include. - src/arith/const_int_bound.cc: delete the vscale() branch in VisitExpr_(CallNode*) (lines 427-430) that returned MakeBound(1, max_val) via GetVScaleValues. vscale() calls now fall back to Everything(op->dtype). Drop scalable_expression.h and the Target::Current() call. Behavioural regression: on VLA targets (SVE / RVV), the analyzer no longer narrows vscale() bounds or proves vscale-bearing inequalities by substitution. The simplifications in rewrite_simplify.cc that call CanProve on vscale expressions (min/max vscale comparison blocks) will fire less often. This is the intended policy change.
…ng users
All consumers of the now-empty arith/scalable_expression.{h,cc} have been
migrated in earlier commits. This commit performs the final deletion and
handles the two remaining legitimate non-proof callers.
src/target/llvm/codegen_aarch64.cc: inline GetVScaleValues at the one
non-proof call site (SetTargetAttributes). The function computed
max_val = largest power-of-two ≤ vector_width/8 from
target.llvm_get_vector_width. The inlined version uses a bit-shift loop
instead of std::pow, avoids building an intermediate vector, and guards
the attribute with max_val > 0.
src/tirx/transform/vectorize_loop.cc: TargetHasVLA and IsVScaleCall are
now file-local anonymous-namespace helpers (added in the recognizer commit).
No further changes needed here.
Delete src/arith/scalable_expression.h and src/arith/scalable_expression.cc.
CMake's file glob drops the .cc automatically; no CMakeLists.txt edit needed.
Post-deletion sweep confirms no remaining references to the deleted symbols
outside the expected file-local helpers.
…of loop Removing the CanProveVscaleExpressionFromKnownValues substitution loop from arith::CanProve and removing const_int_bound's vscale narrow weakens the analyzer for scalable-vector expressions. Tests that relied on these proof paths now fall short. Affected test groups: - TestScalableIndex (test_arith_rewrite_simplify.py): 13 min/max/floordiv/ floormod simplifications that called CanProve on vscale-bearing predicates (e.g. vscale()*4 >= 0) are no longer provable. Marked xfail. - test_simplify_vscale_comparison_with_sve_target (test_arith_simplify.py): 4 direct tests of CanProve on vscale expressions with an SVE target. Marked xfail. - test_simplify_vscale_comparison_without_sve_target (test_arith_simplify.py): Checked for LOG(WARNING) that was in the deleted proof block. Marked xfail. - test_rvv_fast_softmax_vectorizes_exp (test_cpu_reduction.py): end-to-end RVV codegen test that required vscale bounds narrowing to produce scalable vector ops in the scheduled LLVM IR. Marked xfail. All of these regressions are intentional: the policy change is that arith no longer attempts to reason about scalable-vector lengths at the target level.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request removes the target-dependent proof loop for vscale-bearing expressions (CanProveVscaleExpressionFromKnownValues) and deletes the scalable_expression utility files. The helper functions are localized to the files that still require them, and several tests that relied on target-dependent vscale proofs are marked as expected failures. The review feedback suggests preventing potential undefined behavior in a bit-shift loop in codegen_aarch64.cc, adding an explicit cast to avoid narrowing conversion warnings in rewrite_simplify.cc, and removing a redundant namespace qualification in expr.cc.
tqchen
force-pushed
the
tvm-phase-out-scalable-expr
branch
from
aa028ad to
e9f441f
Compare
…ector_width query Outside an active target context Target::Current() returns an undefined Target; passing it to llvm_get_vector_width_fn would crash. Skip the vscale_range attribute when no target is active.
tqchen
force-pushed
the
tvm-phase-out-scalable-expr
branch
from
e9f441f to
7647e45
Compare
tlopex
approved these changes
May 29, 2026
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.
Summary
Phase out
src/arith/scalable_expression.{h,cc}. The arith layer no longer attempts to prove anything about scalable vectors — proofs that depended onTarget::Current()are removed. Scalable vectors remain a first-class concept; arith just doesn't reason about their lengths.Use-site summary
Only 16 call sites total across 7 symbols (9 live, 7 proof-related).
ExtractVscaleFactorarith/rewrite_simplify.cc+ 2 ×tirx/ir/expr.ccIsVScaleCalltirx/op/op.cc+ 1 ×tirx/transform/vectorize_loop.ccContainsVscaleCallarith/rewrite_simplify.cc+ 1 ×s_tir/schedule/ir_comparator.ccTargetHasVLAtirx/transform/vectorize_loop.ccGetVScaleValuestarget/llvm/codegen_aarch64.ccCanProveVscaleExpressionFromKnownValuesSubstituteVScaleWithKnownValueChanges (6 commits)
ExtractVscaleFactorto file-local anonymous-namespace helpers inrewrite_simplify.ccandtirx/ir/expr.cc. Function is small; per-file duplication is cleaner than a shared header.IsVScaleCall/ContainsVscaleCall/TargetHasVLAat call sites (1-3 line predicates, anonymous-namespace per consumer.cc).arith/analyzer.cc(substitution-proof loop) andarith/const_int_bound.cc(vscale branch).vscale()calls fall back toEverything()— no special bound narrowing.scalable_expression.{h,cc}. Inline theGetVScaleValuesbody atcodegen_aarch64.cc(computesmax_val = vector_width / 8floor-rounded to a power of two for the LLVMvscale_rangeattribute).pytest.mark.xfailon 19 tests that relied on the deleted substitution-proof loop.pre-commitline-length cleanup.Compatibility / intentional regression
This is a hard break for any consumer of the deleted symbols. They were already in a private header (
src/arith/scalable_expression.h, not underinclude/).19 tests that proved vscale-bearing inequalities on SVE / RVV are xfailed. The proofs were target-dependent and the new policy is that arith does not attempt them.