Skip to content

Remove non-constant input check from ConvActivationFusion #24525

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

Merged
merged 4 commits into from
Apr 25, 2025

Conversation

edgchen1
Copy link
Contributor

Description

An additional check for non-constant inputs was added to ConvActivationFusion in #20282. This was to avoid fusing an Add in a Conv+Add+Relu that has another non-constant input.

const Node* next_node = &*node.OutputNodesBegin();
// ensure that the target node also has only one input that is not an initializer
const size_t input_edges_total = next_node->GetInputEdgesCount();
int non_const_edges = 0;
for (size_t edge_idx = 0; edge_idx < input_edges_total; ++edge_idx) {
if (!graph_utils::NodeArgIsConstant(graph_viewer.GetGraph(), *next_node->InputDefs()[edge_idx])) {
++non_const_edges;
}
}
if (non_const_edges > 1) {
return nullptr;
} else {
return next_node;
}

However, this check fails to account for implicit inputs and will read past the end of a node's explicit input defs if any implicit inputs are present.

Moreover, this check is no longer necessary after #19470 removed Conv+Add+Relu fusion from ConvActivationFusion.

This change removes the check and some other unused code.

Motivation and Context

Fix #24473.

@edgchen1 edgchen1 merged commit cc72c3a into main Apr 25, 2025
87 of 89 checks passed
@edgchen1 edgchen1 deleted the edgchen1/fix_conv_activation_fusion branch April 25, 2025 01:57
ankitm3k pushed a commit to intel/onnxruntime that referenced this pull request May 12, 2025
…24525)

### Description
<!-- Describe your changes. -->

An additional check for non-constant inputs was added to
ConvActivationFusion in microsoft#20282. This was to avoid fusing an Add in a
Conv+Add+Relu that has another non-constant input.


https://github.com/microsoft/onnxruntime/blob/6c8cb6a6d1993f84fcf4008f468a071c0b73aad3/onnxruntime/core/optimizer/conv_activation_fusion.cc#L26-L39

However, this check fails to account for implicit inputs and will read
past the end of a node's explicit input defs if any implicit inputs are
present.

Moreover, this check is no longer necessary after microsoft#19470 removed
Conv+Add+Relu fusion from ConvActivationFusion.

This change removes the check and some other unused code.

### Motivation and Context
<!-- - Why is this change required? What problem does it solve?
- If it fixes an open issue, please link to the issue here. -->

Fix microsoft#24473.
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.

ORT aborts when loading the attached model
2 participants