Skip to content

[Unity] Ignore R.ExternFunc in EliminateCommonSubexpr #15900

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Hzfengsy merged 1 commit into from
Oct 11, 2023
Merged

[Unity] Ignore R.ExternFunc in EliminateCommonSubexpr #15900

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/relax/transform/eliminate_common_subexpr.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ class SubexprCounter : public ExprVisitor {
if (!(e->IsInstance<VarNode>() || e->IsInstance<DataflowVarNode>() ||
e->IsInstance<GlobalVarNode>() || e->IsInstance<tvm::OpNode>() ||
e->IsInstance<PrimValueNode>() || e->IsInstance<StringImmNode>() ||
e->IsInstance<ShapeExprNode>() ||
e->IsInstance<ShapeExprNode>() || e->IsInstance<ExternFuncNode>() ||
(e.as<ConstantNode>() && (e.as<ConstantNode>()->is_scalar())))) {
// also if e has an impure subexpression, we will not deduplicate it
if (!impurity_detector_.Detect(e)) {
Expand Down
14 changes: 14 additions & 0 deletions tests/python/relax/test_transform_cse.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -276,5 +276,19 @@ def foo(x: R.Tensor((2, 3), dtype="float32"), y: R.Tensor((2, 3), dtype="float32
verify(Before, Expected)


def test_do_not_eliminate_extern_func():
@I.ir_module
class Before:
@R.function(pure=False)
def foo(x: R.Tensor((2, 3), dtype="float32")):
y = R.call_packed("extern_func_name", x, sinfo_args=R.Tensor([2, 3]))
z = R.call_packed("extern_func_name", y, sinfo_args=R.Tensor([2, 3]))
Copy link
Contributor

@kparzysz-quic kparzysz-quic Oct 9, 2023

Choose a reason for hiding this comment

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

If this was commoned before, then something else is probably wrong. This is equivalent to extern_func(extern_func(x)), so there is no opportunity to common anything even if the external function was known to be pure.

Copy link
Contributor Author

@Lunderberg Lunderberg Oct 9, 2023

Choose a reason for hiding this comment

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

Previously, it wasn't a computational step that was being extracted as the common subexpression, but the R.ExternFunc object itself.

@I.ir_module
class Module:
    @R.function(pure=False)
    def foo(x: R.Tensor((2, 3), dtype="float32")):
        gv = R.ExternFunc("extern_func_name")
        y = gv(x, sinfo_args=(R.Tensor((2, 3)),))
        z = gv(y, sinfo_args=(R.Tensor((2, 3)),))
        return z

Copy link
Contributor

@kparzysz-quic kparzysz-quic Oct 9, 2023

Choose a reason for hiding this comment

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

I see. Yeah, that seems unnecessary.

return z

Expected = Before

verify(Before, Expected)


if __name__ == "__main__":
tvm.testing.main()