Skip to content

Conversation

@multiverstack-intellif
Copy link
Contributor

@multiverstack-intellif multiverstack-intellif commented Jan 5, 2023

Much of the index computation is duplicated, so this PR is to do common subexpression analyze when performing cache_index.
And because all block var related computation could be cached, buffer-based cache is somehow too restrictive. So here make it block-based.

A threshold number is needed when identifying a common sub expr, since we only want to cache the most frequently appeared sub expressions. A common use case may be like this: calling tvm.arith.detect_common_subexpr() and determining based on its result whether to perform cache_index and its corresponding threshold.
cc @wrongtest-intellif @Hzfengsy

@tvm-bot
Copy link
Collaborator

tvm-bot commented Jan 5, 2023

Thanks for contributing to TVM! Please refer to the contributing guidelines https://tvm.apache.org/docs/contribute/ for useful information and tips. Please request code reviews from Reviewers by @-ing them in a comment.

Generated by tvm-bot

@multiverstack-intellif multiverstack-intellif changed the title [TIR][Schedule] Improve cache_index to analyze & cache common subexpression [TIR][Schedule] Improve cache_index to cache common subexpressions Jan 10, 2023
@multiverstack-intellif multiverstack-intellif force-pushed the cache branch 5 times, most recently from 66f3b2d to 2a804ef Compare January 12, 2023 15:28
@multiverstack-intellif multiverstack-intellif marked this pull request as ready for review January 13, 2023 01:12
self,
block: Union[BlockRV, str],
storage_scope: str,
cse_thresh: int,
Copy link
Member

Choose a reason for hiding this comment

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

Can we set a default value for cse_thresh? e.g. 0/-1 for not enabling cse?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Can we set a default value for cse_thresh? e.g. 0/-1 for not enabling cse?

OK, I'll set 0 as caching all the index computation for the target block, should be useful for simple cases and expensive in complex cases.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's done. @Hzfengsy

Much of the index computation is duplicated, so this PR is to do common
subexpression analyze when performing cache_index.

A threshold number is needed when identifying a common sub expr, since
we only want to cache the most frequently appeared sub expressions. And
an arith helper function is added to help analyze the target expr. A
common use case may be calling tvm.arith.detect_common_subexpr() and
determining based on its result whether to perform cache_index and its
corresponding threshold.
@wrongtest-intellif wrongtest-intellif merged commit 5e36ae3 into apache:main Jan 20, 2023
fzi-peccia pushed a commit to fzi-peccia/tvm that referenced this pull request Mar 27, 2023
…pache#13700)

Much of the index computation is duplicated, so this PR is to do common
subexpression analyze when performing cache_index.

A threshold number is needed when identifying a common sub expr, since
we only want to cache the most frequently appeared sub expressions. And
an arith helper function is added to help analyze the target expr. A
common use case may be calling tvm.arith.detect_common_subexpr() and
determining based on its result whether to perform cache_index and its
corresponding threshold.

Co-authored-by: Min Chen <chen.min@intellif.com>
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