Skip to content

Conversation

@Lunderberg
Copy link
Contributor

While it is ill-formed for control-flow to occur within a DataflowBlock, it is legal for a DataflowBlock to be contained within a control-flow. Prior to this commit, the FuseOps and FuseOpsByPattern transforms erroneously skipped DataflowBlock instances that were contained within a relax::If node.

This commit updates FuseOps to apply operator fusion to any dataflow block, regardless of whether it is found at the top level of a a Relax function.

While it is ill-formed for control-flow to occur within a
`DataflowBlock`, it is legal for a `DataflowBlock` to be contained
within a control-flow.  Prior to this commit, the `FuseOps` and
`FuseOpsByPattern` transforms erroneously skipped `DataflowBlock`
instances that were contained within a `relax::If` node.

This commit updates `FuseOps` to apply operator fusion to any dataflow
block, regardless of whether it is found at the top level of a a Relax
function.

Co-authored-by: Chris Sullivan <csullivan@octoml.ai>
if (it != graph_.node_map.end()) {
leaf_node = it->second;
} else if (leaf_expr->IsInstance<ConstantNode>() || leaf_expr->IsInstance<ShapeExprNode>() ||
leaf_expr->IsInstance<PrimValueNode>() || leaf_expr->IsInstance<StringImmNode>() ||
Copy link
Member

Choose a reason for hiding this comment

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

Is this intended change? Seems that we should consider sass where constant value(and there may not be a binding pt)

@Lunderberg
Copy link
Contributor Author

Closing to resubmit as a new PR (same issue as mentioned here), due to CI issues.

@Lunderberg Lunderberg closed this May 28, 2024
@Lunderberg Lunderberg deleted the relax_bugfix_operator_fusion_inside_conditional branch May 29, 2024 14:28
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.

2 participants