Skip to content

[CPU] Handle ONNX domain Gelu and HardSigmoid activations in the NCHWc transformer suite#27821

Merged
hariharans29 merged 5 commits intomainfrom
hari/nchwc_transformer_fixes_2
Mar 24, 2026
Merged

[CPU] Handle ONNX domain Gelu and HardSigmoid activations in the NCHWc transformer suite#27821
hariharans29 merged 5 commits intomainfrom
hari/nchwc_transformer_fixes_2

Conversation

@hariharans29
Copy link
Member

Description

As title

Motivation and Context

Tiny continuation to #27691

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Extends the CPU NCHWc transformer pipeline to treat HardSigmoid like the other supported elementwise activations, enabling both (a) reorder-free passthrough in NCHWc subgraphs and (b) fusion into an upstream NCHWc Conv when eligible.

Changes:

  • Add HardSigmoid handling in NchwcTransformerImpl::TransformActivation, including passing alpha/beta via activation_params when fusing into NCHWc Conv.
  • Recognize HardSigmoid nodes in the transformer dispatch list (opset since versions 6 and 22).
  • Add/extend optimizer tests to cover both “no extra reorder” behavior and single-consumer Conv+Activation fusion (including alpha/beta attribute propagation).

Reviewed changes

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

File Description
onnxruntime/test/optimizer/nchwc_optimizer_test.cc Adds HardSigmoid coverage for reorder-avoidance and Conv+activation fusion (including activation_params validation).
onnxruntime/core/optimizer/nchwc_transformer.cc Enables HardSigmoid as a supported activation and fuses it into NCHWc Conv with correct default/custom alpha/beta handling.

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

@hariharans29 hariharans29 requested review from awvwgk and tianleiwu and removed request for awvwgk March 24, 2026 01:41
@hariharans29 hariharans29 requested a review from Copilot March 24, 2026 03:57
@hariharans29 hariharans29 changed the title [CPU] Handle HardSigmoid in the NCHWc transformer suite [CPU] Handle ONNX domain Gelu and HardSigmoid activations in the NCHWc transformer suite Mar 24, 2026
Copy link
Contributor

Copilot AI left a comment

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 1 comment.


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

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

You can commit the suggested changes from lintrunner.

@tianleiwu
Copy link
Contributor

Optimizer Tests (onnxruntime/test/optimizer/nchwc_optimizer_test.cc)

Concern:

  • ⚠️ Missing HardSigmoid in the no-fusion guard test (Nitpick): The new ActivationSingleConsumerConvNoFusion test only covers Gelu (both ONNX and MS-domain) and QuickGelu — activations that should never fuse. This is intentional and correct. However, there's no test for a scenario where HardSigmoid is present but the Conv already has an activation fused (double-fusion prevention). That edge case is covered by the graph_utils::GetNodeAttribute(nchwc_node, "activation") == nullptr guard in the transformer, but there's no explicit test for it. This is a pre-existing gap, not introduced by this PR.

Verdict

APPROVE — Well-structured, minimal change with correct defaults, proper kernel-level support already in place, and thorough test coverage. No functional issues found.

Copy link
Contributor

@tianleiwu tianleiwu left a comment

Choose a reason for hiding this comment

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

Please run lintrunner

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
@hariharans29 hariharans29 enabled auto-merge (squash) March 24, 2026 17:29
@hariharans29 hariharans29 merged commit 1b982dd into main Mar 24, 2026
91 checks passed
@hariharans29 hariharans29 deleted the hari/nchwc_transformer_fixes_2 branch March 24, 2026 20:57
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