Skip to content

Address PadFusion shortcomings#28780

Merged
xadupre merged 3 commits into
mainfrom
yuslepukhin/padfusion_msrc_119779
Jun 4, 2026
Merged

Address PadFusion shortcomings#28780
xadupre merged 3 commits into
mainfrom
yuslepukhin/padfusion_msrc_119779

Conversation

@yuslepukhin

Copy link
Copy Markdown
Contributor

This pull request improves the robustness and correctness of the PadFusion graph optimization in ONNX Runtime by tightening validation on the pads attribute and adding comprehensive test coverage for edge cases. The changes ensure that PadFusion only applies when the pads attribute meets ONNX specification requirements and that it safely handles malformed or unexpected input without crashing.

PadFusion validation improvements:

  • Added checks to ensure PadFusion only applies when the pads attribute has at least four elements and an even length, as required by the ONNX Pad specification. If these conditions are not met, the fusion is skipped.
  • Added logic to verify that if the target node (e.g., Conv) already has a pads attribute, its length matches the expected spatial rank implied by the Pad node. If not, the fusion is skipped to prevent mismatched padding.

Code consistency and safety:

  • Updated types from uint32_t to size_t for variables representing sizes and indices, improving type safety and consistency throughout pad_fusion.cc. [1] [2]

Test coverage enhancements:

  • Added new tests to verify that PadFusion does not fuse and does not crash when:
    • The Pad node's pads attribute or initializer is too short or has an odd length.
    • The pads attribute contains negative values.
    • The child node's pads attribute length does not match the spatial rank implied by the Pad node.
    • The opset-10 attribute form of Pad has a short pads attribute.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This pull request hardens the PadFusion graph rewrite to avoid fusing invalid/malformed Pad configurations (especially around pads length/rank expectations), and adds regression tests to ensure the optimizer safely skips those cases without crashing.

Changes:

  • Tightened pads validation in PadFusion::Apply (minimum length, even length, and target node pads length compatibility).
  • Switched size/index types in pad_fusion.cc from uint32_t to size_t and adjusted indexing accordingly.
  • Added new optimizer tests covering short/odd pads, negative pads, and mismatched child pads rank (including opset-10 attribute-form Pad).

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
onnxruntime/core/optimizer/pad_fusion.cc Adds stricter pads validation and guards to prevent unsafe fusion when Pad/target padding attributes are malformed or incompatible.
onnxruntime/test/optimizer/graph_transform_test.cc Adds regression tests ensuring PadFusion safely bails out (no fusion, no crash) for multiple malformed pads edge cases.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread onnxruntime/core/optimizer/pad_fusion.cc Outdated

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

@xadupre
xadupre merged commit eded2c7 into main Jun 4, 2026
87 checks passed
@xadupre
xadupre deleted the yuslepukhin/padfusion_msrc_119779 branch June 4, 2026 07:01
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.

3 participants