Skip to content

feat(hipdnn): add GPU batchnorm backward reference - #12520

Merged
BalintCsala merged 10 commits into
ROCm:developfrom
StreamHPC:users/BalintCsala/batchnorm-backward-gpu-ref
Oct 9, 2026
Merged

BalintCsala merged 10 commits into
ROCm:developfrom
StreamHPC:users/BalintCsala/batchnorm-backward-gpu-ref

Conversation

@BalintCsala

Copy link
Copy Markdown
Member

Motivation

We need a reference batchnorm backward implementation on the GPU for faster tests.

Closes #12519

Technical Details

Kernel is compiled with HipRTC and supports fp32, fp16 and bfp16.

Test Plan

Tested against the CPU implementation.

Test Result

Tests passes locally.

Submission Checklist

@therock-pr-bot

therock-pr-bot Bot commented Sep 24, 2026 •

Copy link
Copy Markdown

✅ All Checks Passed — Ready for Review

Check Status Details
📝 PR Description ✅ Pass —
⛔ Forbidden Files ✅ Pass —
🧪 Unit Test ✅ Pass —
🔎 pre-commit ✅ Pass —
🚫 Draft PR 🔜 To Be Enabled —
🚩 Feature Flag 🔜 To Be Enabled —
📊 Code Coverage 🔜 To Be Enabled —
🤖 therock-pr-bot ✅ Pass —

🎉 All checks passed! This PR is ready for review.

📖 Need help? See the Policy FAQ for details on every check and how to fix failures.

🙋 Wish to Override Policy?

@therock-pr-bot

therock-pr-bot Bot commented Sep 24, 2026 •

Copy link
Copy Markdown

🎉 All checks passed! This PR is ready for review.

@BalintCsala
BalintCsala force-pushed the users/BalintCsala/batchnorm-backward-gpu-ref branch from 8e3935f to 446477c Compare September 24, 2026 10:43
@brentmaas
brentmaas requested review from saikubairkota and a balanced review from Copilot September 24, 2026 12:35

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.

Copilot review overview

🟢 Approval recommended

The only unresolved finding is a non-blocking test-coverage nit.

Review effort: Balanced
Findings: None

What changed in this PR

Adds a HipRTC GPU reference implementation for batch normalization backward, supporting FP32, FP16, BFP16, and saved or calculated statistics.

Changes:

  • Adds the backward kernel, launch API, arguments, and validation.
  • Adds CPU-comparison integration tests and build registration.
  • Supports multiple tensor ranks and layouts.
File Description
dnn-providers/​integration-tests/​tests/​gpu-ref/​TestGpuFpReferenceBatchnormBwd.cpp Adds CPU/GPU correctness tests; larger calculated-statistics coverage is suggested.
dnn-providers/​integration-tests/​tests/​CMakeLists.txt Registers the new tests.
dnn-providers/​integration-tests/​gpu-ref/​src/​GpuFpReferenceBatchnorm.cpp Implements kernel compilation and launch.
dnn-providers/​integration-tests/​gpu-ref/​kernels/​types/​GpuRefBatchnormArgs.h Defines backward kernel arguments.
dnn-providers/​integration-tests/​gpu-ref/​kernels/​CMakeLists.txt Packages the new kernel.
dnn-providers/​integration-tests/​gpu-ref/​kernels/​batchnorm/​GpuRefBatchnormBwd.cpp Implements backward calculations and reductions.
dnn-providers/​integration-tests/​gpu-ref/​include/​hipdnn-gpu-ref/​GpuFpReferenceBatchnorm.hpp Adds the backward API and validation.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@BalintCsala
BalintCsala requested a review from a team as a code owner September 28, 2026 14:42
@BalintCsala
BalintCsala force-pushed the users/BalintCsala/batchnorm-backward-gpu-ref branch from a33924e to 90cc000 Compare September 28, 2026 14:44
@BalintCsala
BalintCsala force-pushed the users/BalintCsala/batchnorm-backward-gpu-ref branch from 2b977f9 to 2cfcd6b Compare September 30, 2026 15:05
@therock-pr-bot

therock-pr-bot Bot commented Oct 5, 2026

Copy link
Copy Markdown

Pre-commit check failed

⛔ pre-commit failed

Please run locally:

  • python -m pip install pre-commit
  • pre-commit install
  • pre-commit run --all-files --show-diff-on-failure

This repo uses .pre-commit-config.yaml.

@BalintCsala
BalintCsala force-pushed the users/BalintCsala/batchnorm-backward-gpu-ref branch from 49f78d7 to 6211a09 Compare October 5, 2026 14:49
@BalintCsala
BalintCsala force-pushed the users/BalintCsala/batchnorm-backward-gpu-ref branch from 6211a09 to a78b447 Compare October 5, 2026 17:05

@adickin-amd adickin-amd 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.

A few things to look at but overall LGTM

@BalintCsala
BalintCsala enabled auto-merge (squash) October 6, 2026 09:12
@BalintCsala BalintCsala mentioned this pull request Oct 6, 2026
1 task done
@JeniferC99

Copy link
Copy Markdown
Contributor

Cancelling the remaining gfx950 jobs on this run so the TheRock submodule bump PRs can be prioritized on the gfx950 queue.

This change does not target gfx950, and gfx950 was the only work still outstanding. Jobs that already finished are unchanged. Re-run failed jobs if you need gfx950 results before merge.

@BalintCsala
BalintCsala disabled auto-merge October 7, 2026 11:29
@BalintCsala
BalintCsala enabled auto-merge (squash) October 7, 2026 14:45
@BalintCsala
BalintCsala merged commit d8f9550 into ROCm:develop Oct 9, 2026
97 checks passed
@assistant-librarian

Copy link
Copy Markdown
Contributor

TheRock Submodule Bump Activity

Newest first

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

GPU batchnorm backward gpu reference implementation

5 participants