Skip to content

Conversation

gedoensmax
Copy link
Contributor

Description

FIx:

  • Multiples Convs into an Add+Relu will fuse the op although intermediates are needed @tianleiwu for viz
    image
  • Also fixes an issue with Shape Initializers Merge as input, that occurs when the input initializer is the same across multiple nodes but not all nodes are Shape nodes. @er3x3 for viz

@tianleiwu
Copy link
Contributor

/azp run Windows ARM64 QNN CI Pipeline,Windows x64 QNN CI Pipeline,Windows CPU CI Pipeline,Windows GPU CI Pipeline,Windows GPU TensorRT CI Pipeline,ONNX Runtime Web CI Pipeline,Linux CPU CI Pipeline,Linux CPU Minimal Build E2E CI Pipeline,Linux GPU CI Pipeline,Linux GPU TensorRT CI Pipeline

@tianleiwu
Copy link
Contributor

/azp run Linux OpenVINO CI Pipeline,Linux QNN CI Pipeline,MacOS CI Pipeline,orttraining-amd-gpu-ci-pipeline,orttraining-linux-ci-pipeline,orttraining-linux-gpu-ci-pipeline,orttraining-ortmodule-distributed,onnxruntime-binary-size-checks-ci-pipeline,Big Models

Copy link

Azure Pipelines successfully started running 10 pipeline(s).

Copy link

Azure Pipelines successfully started running 9 pipeline(s).

@tianleiwu
Copy link
Contributor

/azp run Windows ARM64 QNN CI Pipeline,Windows x64 QNN CI Pipeline,Windows CPU CI Pipeline,Windows GPU CI Pipeline,Windows GPU TensorRT CI Pipeline,ONNX Runtime Web CI Pipeline,Linux CPU CI Pipeline,Linux CPU Minimal Build E2E CI Pipeline,Linux GPU CI Pipeline,Linux GPU TensorRT CI Pipeline

@tianleiwu
Copy link
Contributor

/azp run Linux OpenVINO CI Pipeline,Linux QNN CI Pipeline,MacOS CI Pipeline,orttraining-amd-gpu-ci-pipeline,orttraining-linux-ci-pipeline,orttraining-linux-gpu-ci-pipeline,orttraining-ortmodule-distributed,onnxruntime-binary-size-checks-ci-pipeline,Big Models

Copy link

Azure Pipelines successfully started running 9 pipeline(s).

Copy link

Azure Pipelines successfully started running 10 pipeline(s).

@gedoensmax gedoensmax force-pushed the fix_shape_conv_fuse_opt branch from 584c38a to 6f0e20d Compare April 23, 2024 13:26
@tianleiwu
Copy link
Contributor

/azp run Windows ARM64 QNN CI Pipeline,Windows x64 QNN CI Pipeline,Windows CPU CI Pipeline,Windows GPU CI Pipeline,Windows GPU TensorRT CI Pipeline,ONNX Runtime Web CI Pipeline,Linux CPU CI Pipeline,Linux CPU Minimal Build E2E CI Pipeline,Linux GPU CI Pipeline,Linux GPU TensorRT CI Pipeline

@tianleiwu
Copy link
Contributor

/azp run Linux OpenVINO CI Pipeline,Linux QNN CI Pipeline,MacOS CI Pipeline,orttraining-amd-gpu-ci-pipeline,orttraining-linux-ci-pipeline,orttraining-linux-gpu-ci-pipeline,orttraining-ortmodule-distributed,onnxruntime-binary-size-checks-ci-pipeline,Big Models,Android CI Pipeline

@tianleiwu
Copy link
Contributor

/azp run iOS CI Pipeline,ONNX Runtime React Native CI Pipeline

Copy link

Azure Pipelines successfully started running 2 pipeline(s).

Copy link

Azure Pipelines successfully started running 10 pipeline(s).

1 similar comment
Copy link

Azure Pipelines successfully started running 10 pipeline(s).

@tianleiwu
Copy link
Contributor

/azp run Linux GPU CI Pipeline

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@tianleiwu tianleiwu merged commit b4e5075 into microsoft:main Apr 23, 2024
TedThemistokleous pushed a commit to TedThemistokleous/onnxruntime that referenced this pull request May 7, 2024
FIx:
- Multiples Convs into an Add+Relu will fuse the op although intermediates are needed

![image](https://github.com/microsoft/onnxruntime/assets/44298237/0c85a30c-5f41-4e62-ae2e-f41eada6c2c3)
- Also fixes an issue with Shape Initializers Merge as input, that
occurs when the input initializer is the same across multiple nodes but
not all nodes are Shape nodes.
edgchen1 added a commit that referenced this pull request Apr 25, 2025
### Description
<!-- Describe your changes. -->

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.


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

2 participants