Skip to content

[Unity][Bugfix] Handle symbolic matching with non-structural match#15994

Merged
Hzfengsy merged 2 commits intoapache:unityfrom
Lunderberg:unity_fuse_tir_dynamic_matching
Nov 30, 2023
Merged

[Unity][Bugfix] Handle symbolic matching with non-structural match#15994
Hzfengsy merged 2 commits intoapache:unityfrom
Lunderberg:unity_fuse_tir_dynamic_matching

Conversation

@Lunderberg
Copy link
Contributor

Prior to this commit, FuseTIR required the symbolic shape of an argument to be a structural match to the symbolic shape of a parameter. As a result, erroneous failures to match would occur when the dynamic shape parameter contains expressions, and the static shape argument does not. For example, a parameter [n, m, n*m] is not structurally matched to an argument [16, 16, 256], but is a valid substitution. This commit updates FuseTIR to handle these cases.

@masahi
Copy link
Member

masahi commented Oct 30, 2023

cc @Hzfengsy

LOG(FATAL) << "Failed to match PrimExpr " << param << " with " << arg;
}
VisitExpr(param, arg);
must_prove_ = arith::Analyzer().Simplify(Substitute(must_prove_, *var_remap_));
Copy link
Member

Choose a reason for hiding this comment

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

would be useful to generate analyzer once and reuse multiple times when possibe

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good call, and updated to be outside of the SymbolicMatcher.

Prior to this commit, `FuseTIR` required the symbolic shape of an
argument to be a structural match to the symbolic shape of a
parameter.  As a result, erroneous failures to match would occur when
the dynamic shape parameter contains expressions, and the static shape
argument does not.  For example, a parameter `[n, m, n*m]` is not
structurally matched to an argument `[16, 16, 256]`, but is a valid
substitution.  This commit updates `FuseTIR` to handle these cases.
@Lunderberg Lunderberg force-pushed the unity_fuse_tir_dynamic_matching branch from e2a476e to 567ac75 Compare November 29, 2023 18:29
@Lunderberg
Copy link
Contributor Author

Rebased onto unity to rerun CI, make sure no conflicts have arisen in the past few weeks.

@Hzfengsy Hzfengsy merged commit 6844348 into apache:unity Nov 30, 2023
@Lunderberg Lunderberg deleted the unity_fuse_tir_dynamic_matching branch November 30, 2023 14:17
Archermmt pushed a commit to Archermmt/tvm that referenced this pull request Dec 18, 2023
…pache#15994)

* [Unity][Bugfix] Handle symbolic matching with non-structural match

Prior to this commit, `FuseTIR` required the symbolic shape of an
argument to be a structural match to the symbolic shape of a
parameter.  As a result, erroneous failures to match would occur when
the dynamic shape parameter contains expressions, and the static shape
argument does not.  For example, a parameter `[n, m, n*m]` is not
structurally matched to an argument `[16, 16, 256]`, but is a valid
substitution.  This commit updates `FuseTIR` to handle these cases.

* Avoid repeated initialization of arith::Analyzer
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.

4 participants