Skip to content

Conversation

@cchung100m
Copy link
Contributor

@cchung100m cchung100m commented Nov 23, 2025

Hi Commiters,

This PR is trying to fix issues #17876. Any suggestions would be appreciated if you are available.

Root Cause

VMShapeLower crashed when processing ShapeExpr containing composite PrimExpr that weren't computed yet.

Solution

Modified VisitExpr_(const ShapeExprNode* op) in vm_shape_lower.cc to:

1. Mark uncomputed variables as ready for computation
2. Trigger EmitOutstandingPrimExprCompute() to resolve the dependency chain
3. Ensure all expressions are computed before calling MakeSymbolicShapeArg

Added test case: test_composite_shape_expression_fix() to prevent future occurrences.

symbolizing the composite PrimExpr in the pipeline: canonicalization

@cchung100m cchung100m changed the title [#17876]Fix TVM crashes with default relax pipeline when opt_level=1: InternalError: Check failed: (slot->value_computed) is false [Relax][Backend] Fix TVM crashes with default relax pipeline when opt_level=1: InternalError: Check failed: (slot->value_computed) is false Nov 23, 2025
@cchung100m cchung100m marked this pull request as ready for review November 25, 2025 15:58
@tlopex tlopex self-assigned this Nov 28, 2025
if (it != slot_map_.end() && !it->second->value_computed) {
// If it's a variable, mark it as ready for computation
if (expr.as<tir::VarNode>()) {
it->second->value_computed = true;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Marking arbitrary tir::VarNode instances as value_computed = true in VisitExpr_ is incorrect. In VMShapeLower, only Relax ShapeVars (from function parameters/match_cast) or IntImm constants can be safely marked as computed, because only these have runtime values accessible to the VM.
I think correct way is to fix this earlier in the pipeline (shape inference/canonicalization) by symbolizing composite PrimExpr (introduce Relax ShapeVars) or simplifying them before VMShapeLower.

Copy link
Contributor Author

@cchung100m cchung100m Dec 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @tlopex

Thanks for the suggestion.
Should we extend ComputePrimValue() to handle ShapeExpr nodes with compound PrimExpr?
https://github.com/apache/tvm/blob/main/src/relax/transform/compute_prim_value.cc#L65

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, ComputePrimValue is intended only for evaluating statically evaluable PrimExpr into IntImm (constant folding), so I think extending ComputePrimValue would not address the root issue.
The real problem is that VMShapeLower cannot consume composite PrimExpr directly. The correct solution here should be to canonicalize ShapeExpr earlier by introducing a Relax ShapeVar binding for any non-trivial PrimExpr.

Just like:

# 1. Compute the symbolic value first (Canonicalization)
s1 = R.prim_value(n + 1)

# 2. Pass the computed var to the shape (VMShapeLower is happy now)
lv = R.call_tir(cls.func, (x,), R.shape([s1]), dtype="float32")

@cchung100m cchung100m marked this pull request as draft December 2, 2025 12:37
@cchung100m cchung100m force-pushed the issue-17876 branch 2 times, most recently from 533de8a to b98b5a7 Compare December 2, 2025 13:07
@cchung100m cchung100m force-pushed the issue-17876 branch 19 times, most recently from b492829 to cc57139 Compare December 18, 2025 14:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants