Skip to content
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.

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

[TIR] Fix reverse_compute_at for trivial region with trivial block var #11234

Merged
merged 2 commits into from
May 8, 2022

Conversation

vinx13
Copy link
Member

@vinx13 vinx13 commented May 6, 2022

This PR fixed compute_at / reverse_compute_at when block access regions contain constants (e.g. A[0, vi, vj]).
Background:
#11187 added additional simplification when creating prim func from TE. This may result in block var with constant binding values simplified.

cc @junrushao1994 @spectrometerHBH @zxybazh @shingjan

Copy link
Contributor

@shingjan shingjan left a comment

Choose a reason for hiding this comment

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

LGTM. Can confirm that this PR fixed my repro on meta_schedule with LLVM. Thanks! @vinx13. BTW do you have any insight on why this issue is only happening on LLVM but not CUDA?

@vinx13
Copy link
Member Author

vinx13 commented May 6, 2022

CUDA will call cache_write to introduce a cache block first, so the block used for reverse_compute_at is different, and the bindings we are dealing with might also be different

Copy link
Member

@zxybazh zxybazh left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks for the quick fix and thanks to @shingjan for reporting this issue!

@junrushao
Copy link
Member

Reading the cryptic error message, it seems that the IntervalSet sometimes is used to represent handle vs int, so it cans CanProveEqual - a bit weird to me, but definitely fixable with more guards

  16: _M_invoke
        at /usr/include/c++/7/bits/std_function.h:316
  15: operator()
        at /workspace/src/te/operation/compute_op.cc:216
  14: tvm::arith::IntSetAnalyzer::operator()(tvm::PrimExpr const&, tvm::runtime::Map<tvm::tir::Var, tvm::arith::IntSet, void, void> const&)
        at /workspace/src/arith/int_set.cc:521
  13: tvm::arith::IntSetAnalyzer::Impl::Eval(tvm::PrimExpr const&, tvm::runtime::Map<tvm::tir::Var, tvm::arith::IntSet, void, void> const&) const
        at /workspace/src/arith/int_set.cc:509
  12: tvm::arith::IntervalSetEvaluator::Eval(tvm::PrimExpr const&)
        at /workspace/src/arith/int_set.cc:351
  11: tvm::tir::ExprFunctor<tvm::arith::IntervalSet (tvm::PrimExpr const&)>::VisitExpr(tvm::PrimExpr const&)
        at /workspace/include/tvm/tir/expr_functor.h:114
  10: tvm::NodeFunctor<tvm::arith::IntervalSet (tvm::runtime::ObjectRef const&, tvm::tir::ExprFunctor<tvm::arith::IntervalSet (tvm::PrimExpr const&)>*)>::operator()(tvm::runtime::ObjectRef const&, tvm::tir::ExprFunctor<tvm::arith::IntervalSet (tvm::PrimExpr const&)>*) const
        at /workspace/include/tvm/node/functor.h:97
  9: tvm::tir::ExprFunctor<tvm::arith::IntervalSet (tvm::PrimExpr const&)>::InitVTable()::{lambda(tvm::runtime::ObjectRef const&, tvm::tir::ExprFunctor<tvm::arith::IntervalSet (tvm::PrimExpr const&)>*)#8}::_FUN(tvm::runtime::ObjectRef const&, tvm::tir::ExprFunctor<tvm::arith::IntervalSet (tvm::PrimExpr const&)>*)
        at /workspace/include/tvm/tir/expr_functor.h:171
  8: tvm::tir::ExprFunctor<tvm::arith::IntervalSet (tvm::PrimExpr const&)>::InitVTable()::{lambda(tvm::runtime::ObjectRef const&, tvm::tir::ExprFunctor<tvm::arith::IntervalSet (tvm::PrimExpr const&)>*)#8}::operator()(tvm::runtime::ObjectRef const&, tvm::tir::ExprFunctor<tvm::arith::IntervalSet (tvm::PrimExpr const&)>*) const
        at /workspace/include/tvm/tir/expr_functor.h:171
  7: tvm::arith::IntervalSetEvaluator::VisitExpr_(tvm::tir::AddNode const*)
        at /workspace/src/arith/int_set.cc:383
  6: tvm::arith::IntervalSet tvm::arith::IntervalSetEvaluator::VisitBinaryExpr_<tvm::tir::Add, tvm::tir::AddNode>(tvm::tir::AddNode const*)
        at /workspace/src/arith/int_set.cc:493
  5: tvm::arith::IntervalSet tvm::arith::Combine<tvm::tir::Add>(tvm::arith::Analyzer*, tvm::arith::IntervalSet, tvm::arith::IntervalSet)
        at /workspace/src/arith/int_set.cc:126
  4: tvm::arith::IntervalSetNode::IsSinglePoint() const
        at /workspace/src/arith/interval_set.h:67
  3: tvm::arith::Analyzer::CanProveEqual(tvm::PrimExpr const&, tvm::PrimExpr const&)
        at /workspace/src/arith/analyzer.cc:107
  2: tvm::operator==(tvm::PrimExpr, tvm::PrimExpr)
        at /workspace/src/tir/op/op.cc:501
  1: tvm::equal(tvm::PrimExpr, tvm::PrimExpr, tvm::Span)
        at /workspace/src/tir/op/op.cc:503
  0: tvm::BinaryOpMatchTypes(tvm::PrimExpr&, tvm::PrimExpr&, tvm::Span)
        at /workspace/src/tir/op/op.cc:163
  File "/workspace/src/tir/op/op.cc", line 163
TVMError: Cannot match type handle vs int32

@wrongtest-intellif
Copy link
Contributor

Reading the cryptic error message, it seems that the IntervalSet sometimes is used to represent handle vs int, so it cans CanProveEqual - a bit weird to me, but definitely fixable with more guards

In IntervalSet, positive and negative infinity are two special values with type handle, and intset.min() and intset.max() should check whether be infinity, can not directly assume they are of integer type.

@junrushao
Copy link
Member

junrushao commented May 7, 2022

That makes a lot of sense @wrongtest

@Hzfengsy Hzfengsy merged commit d049079 into apache:main May 8, 2022
shtinsa pushed a commit to Deelvin/tvm that referenced this pull request May 17, 2022
apache#11234)

* [TIR] Fix reverse_compute_at for trivial region with trivial block var

* Prevent handle arithmetics
SebastianBoblest pushed a commit to SebastianBoblest/tvm that referenced this pull request May 27, 2022
apache#11234)

* [TIR] Fix reverse_compute_at for trivial region with trivial block var

* Prevent handle arithmetics
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.

6 participants