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

Improve comments #4633

Merged
merged 2 commits into from
Jan 6, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
17 changes: 9 additions & 8 deletions src/relay/pass/fuse_ops.cc
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ namespace relay {
/*
Note on Fusing algorithm:

The main challenge of genenral fusor is to handle possible diamond shape branches,
The main challenge of general fusor is to handle possible diamond shape branches,
in the following graph, conv2d can be fused to elemwise add.

conv2d
Expand All @@ -51,8 +51,9 @@ namespace relay {
elemwise add
|

However, at the point of conv2d we do not necessarily know that all its future path
will merge at the elemwise add. The new fusor algorithm applies post-dominator analysis.
However, at the point of conv2d we do not necessarily know that all the future paths
will merge at the elemwise add. The fusion algorithm applies post-dominator analysis.

The immediate post-dominator of a node defined by the closest node where all the future path goes into.
In the above case, the elemwise add is the post-dominator of conv2d. The general algorithm is as follows:

Expand All @@ -67,7 +68,7 @@ namespace relay {
immediate post dominator. It has to check the following things:

- CheckPath: check all the path between a node and its immediate post-dominator
satiesfies the fuse condition.
satisfies the fuse condition.
- Note that these intermediate node can already be fused with another nodes, the algorithm
will still run correctly.
- CommitFuse: mark all the nodes between source and post-dominator as the same group.
Expand All @@ -84,7 +85,7 @@ static const Op& stop_fusion_op = Op::Get("annotation.stop_fusion");
* \brief Indexed data flow graph in forward direction.
* This is a temporary data structure used for operator fusion analysis.
*
* This data structure only captures the dataflow fragement and
* This data structure only captures the dataflow fragment and
* could ignore blocks like let by simply ordering each dataflow block
* and mark the output node as extern_ref;
*/
Expand Down Expand Up @@ -287,7 +288,7 @@ class IndexedForwardGraph::Creator : private ExprVisitor {
void VisitExpr_(const TupleGetItemNode* op) final {
auto tuple_type = op->tuple->checked_type().as<TupleTypeNode>();
CHECK(tuple_type);
// when TVM lowers a fused function, it expects all arguments to be a Tensor or
// When TVM lowers a fused function, it expects all arguments to be a Tensor or
// a tuple containing only Tensors. But this tuple may contain a reference or
// another tuple. To avoid modifying codegen logic, we do not allow fusing through this node
// if the tuple contains such non Tensor fields. However, all fields will be recursively
Expand Down Expand Up @@ -391,10 +392,10 @@ class DominatorTree {
/*!
* \brief compute a post dominator relation for a given dataflow graph.
* \param arena The arena used for node allocation.
* \param graph The graph to be analyze.
* \param graph The graph to be analyzed.
* \return The dominator tree of the graph.
* \note This algorithm makes use of the fact that graph is DAG,
* and runs a single pass algorithm via LCA.
* and runs a single pass algorithm via LCA (Least Common Ancestor)
*/
static DominatorTree PostDom(common::Arena* arena,
const IndexedForwardGraph& graph);
Expand Down
2 changes: 1 addition & 1 deletion src/relay/pass/well_formed.cc
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ namespace tvm {
namespace relay {


//! brief make sure each Var is bind at most once.
//! brief make sure each Var is bound at most once in a scope.
class WellFormedChecker : private ExprVisitor, PatternVisitor {
bool well_formed = true;

Expand Down