Skip to content

Conversation

@Hzfengsy
Copy link
Member

The current fuse_ops.cc contains the following parts:

  1. IndexedForwardGraph and DominatorTree which are used for graph partitioning
  2. A Relay Expr visitor to create the DominatorTree
  3. A Relay Expr mutator to fuse the ops

This PR separates the graph partitioning code from fuse_ops.cc and moves it to the analysis folder, for:

  1. Better code organization and readability as the graph partitioning code is quite long and not directly related to the fusion mutator
  2. Possible reuse opportunities for other fusion passes in Relax

NOTE: we won't bring relax fusion in main branch for now, but this pr is still reasonable for main.

cc @tqchen

The current `fuse_ops.cc` contains the following parts:
1. `IndexedForwardGraph` and `DominatorTree` which are used for graph partitioning
2. A Relay Expr visitor to create the `DominatorTree`
3. A Relay Expr mutator to fuse the ops

This PR separates the graph partitioning code from `fuse_ops.cc` and moves it to the analysis folder, for:
1. Better code organization and readability as the graph partitioning code is quite long and not directly related to the fusion mutator
2. Possible reuse opportunities for other fusion passes in Relax

NOTE: we won't bring relax fusion in `main` branch for now, but this pr is still reasonable for `main`.
@tvm-bot
Copy link
Collaborator

tvm-bot commented Feb 12, 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

@github-actions github-actions bot requested a review from tqchen February 12, 2023 06:41
Copy link
Member

@junrushao junrushao left a comment

Choose a reason for hiding this comment

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

looks like the only dependency to relay is op_attr_types, so shall we instead move them to src/ir/?

@tqchen
Copy link
Member

tqchen commented Feb 12, 2023

I think it is a good positive change overall. We can think about future reorganizations as followup. Perhaps some extra namespace for related support data structures. Going to merge since this is a positive internal refactor.

@tqchen tqchen merged commit a49a7fe into apache:main Feb 12, 2023
@Hzfengsy Hzfengsy deleted the refactor_fuse_ope branch November 5, 2023 09:43
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