Repository navigation
Enable BF16 for opset 28 operators - #32378
Justin Chu (justinchuby) wants to merge 18 commits into
Conversation
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 6e64e522-b07f-4c1c-abf1-3985268beb7e
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
|
einsum for BFloat16 is not going to be very efficient since it is not using mlas. |
True. I will create a follow up PR for this |
There was a problem hiding this comment.
🟡 Changes recommended
Legacy Unique coverage is broadened, generated kernel documentation and OneHot coverage are incomplete, and BF16 Einsum matmul ignores CPU parallelism.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Enables opset-28 BF16 execution across CPU and selected CUDA operators while preserving older registration boundaries.
Changes:
- Adds opset-28 CPU/CUDA kernel registrations.
- Implements BF16 Celu, Unique, and Einsum execution paths.
- Adds focused BF16 provider tests.
File summaries
| File | Description |
|---|---|
onnxruntime/test/providers/cpu/tensor/unique_op_test.cc |
Tests BF16 Unique. |
onnxruntime/test/providers/cpu/tensor/reverse_sequence_test.cc |
Tests BF16 ReverseSequence. |
onnxruntime/test/providers/cpu/tensor/onehot_op_test.cc |
Tests BF16 OneHot. |
onnxruntime/test/providers/cpu/tensor/compress_op.test.cc |
Tests BF16 Compress. |
onnxruntime/test/providers/cpu/math/einsum_test.cc |
Tests BF16 Einsum paths. |
onnxruntime/test/providers/cpu/activation/activation_op_test.cc |
Tests new Celu types. |
onnxruntime/core/providers/webgpu/webgpu_execution_provider.cc |
Splits WebGPU Einsum registration. |
onnxruntime/core/providers/webgpu/math/einsum.cc |
Registers WebGPU Einsum v28. |
onnxruntime/core/providers/js/operators/einsum.cc |
Splits JS Einsum registration. |
onnxruntime/core/providers/js/js_execution_provider.cc |
Registers JS Einsum v28. |
onnxruntime/core/providers/cuda/tensor/reverse_sequence.cc |
Adds CUDA BF16 ReverseSequence. |
onnxruntime/core/providers/cuda/tensor/onehot.cu |
Instantiates CUDA BF16 OneHot. |
onnxruntime/core/providers/cuda/tensor/onehot.cc |
Registers CUDA BF16 OneHot. |
onnxruntime/core/providers/cuda/tensor/compress.cc |
Splits CUDA Compress registration. |
onnxruntime/core/providers/cuda/math/einsum.cc |
Caps CUDA Einsum v28 types. |
onnxruntime/core/providers/cuda/cuda_execution_provider.cc |
Registers CUDA opset-28 kernels. |
onnxruntime/core/providers/cpu/tensor/unique.cc |
Implements CPU BF16 Unique. |
onnxruntime/core/providers/cpu/tensor/reverse_sequence.cc |
Splits CPU ReverseSequence registration. |
onnxruntime/core/providers/cpu/tensor/onehot.cc |
Registers CPU BF16 OneHot. |
onnxruntime/core/providers/cpu/tensor/compress.cc |
Splits CPU Compress registration. |
onnxruntime/core/providers/cpu/math/einsum.cc |
Registers CPU BF16 Einsum. |
onnxruntime/core/providers/cpu/math/einsum_utils/einsum_auxiliary_ops.cc |
Implements BF16 Einsum helpers. |
onnxruntime/core/providers/cpu/cpu_execution_provider.cc |
Registers CPU opset-28 kernels. |
onnxruntime/core/providers/cpu/activation/activations.h |
Implements FP16/BF16 Celu. |
onnxruntime/core/providers/cpu/activation/activations.cc |
Registers Celu v28 types. |
Review details
- Files reviewed: 25/25 changed files
- Comments generated: 4
- Review effort level: Balanced
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
/azp run Linux Android Emulator QNN CI Pipeline |
|
Azure Pipelines will not run the associated pipelines, because the pull request was updated after the run command was issued. Review the pull request again and issue a new run command. |
|
/azp run Linux Android Emulator QNN CI Pipeline |
|
Azure Pipelines will not run the associated pipelines, because the pull request was updated after the run command was issued. Review the pull request again and issue a new run command. |
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
|
/azp run Linux Android Emulator QNN CI Pipeline |
|
Azure Pipelines will not run the associated pipelines, because the pull request was updated after the run command was issued. Review the pull request again and issue a new run command. |
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
|
/azp run Linux Android Emulator QNN CI Pipeline |
|
Azure Pipelines will not run the associated pipelines, because the pull request was updated after the run command was issued. Review the pull request again and issue a new run command. |
|
/azp run Linux Android Emulator QNN CI Pipeline |
|
Azure Pipelines will not run the associated pipelines, because the pull request was updated after the run command was issued. Review the pull request again and issue a new run command. |
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Warning
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
Copilot review overview
Review effort: Lite
Findings: 1
Open (5)
This BF16 Einsum test will fail under CUDA builds because CUDA Einsum v28 is registered without… · New The PR title/summary focuses on opset-28 BF16 enablement, but the generated docs update introduces… · New The accelerated BF16 MatMul path materializes full-size float buffers for all batches… · New The BF16 ReduceSum adapter allocatesfloat_inputusinginput.Shape()even when… · New This build-time type guard always checksEnabledUniqueDataTypes28, even though the kernel is… · New
Resolved since last review (4)
This is the only BF16 OneHot execution test, but it uses int64 indices/depth and a nonzero…BFloat16is being added to theALL_OPSETStype list, while both the new[11, 27]registration… The BF16 matmul fallback discards the supplied thread pool and executes the full O(batch·M·N·K)… These CPU/CUDA registration changes are not reflected indocs/OperatorKernels.md; for example,…
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
### Description <!-- Describe your changes. --> Reintegrate the ONNX 1.23 base plumbing originally introduced by #32359 and reverted by #33073, now using ONNX **1.23.2**. - Coordinate source/submodule, FetchContent, vcpkg, and Python dependency pins and raise the relevant opset ceilings to 28. - Restore Float6 public element-type mappings and conversion coverage, preserving subsequent independent type-support changes. - Update existing-type Cast registrations, CPU/CUDA Mod boundaries, signed BitShift support, provider compatibility, backend filters/tolerances, and operator documentation. - Materialize the removed upstream node-test corpus with exact-version and minimum-case guards, and cover backend serialization and released-opset policy. - Keep matching FetchContent/vcpkg compatibility patches; remove the released-opset correction now fixed upstream in 1.23.2. - Address review feedback: OpenVINO rejects BitShift-28; CPU/CUDA legacy SpaceToDepth kernels stop at 27; DirectML accepts only safe DCR; unverified graph providers defer SpaceToDepth-28. Full builds use ONNX function expansion for CRD, with literal-output and routing regressions. Avoid creating unsupported SpaceToDepth-28 nodes in the post-expansion Slice/Concat fusion. - Use shared ABI-safe tensor-element conversion in the native ONNX node runner, fixing INT2/UINT2/FLOAT8E8M0 corpus decoding after the main merge. ### 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. --> This is a redo of #32359 after its rollback in #33073. ONNX 1.23.2 includes the external tensor read/re-save fix from onnx/onnx#8472 for onnx/onnx#8471 and the released-opset metadata fix from onnx/onnx#8544 for onnx/onnx#8529. The redundant local `LastReleaseVersionMap` patch has been removed. Remaining FetchContent/vcpkg patches are byte-identical and apply against the pinned release. **Scope:** This PR establishes dependency/runtime base integration. Full per-operator opset-28 support remains separate follow-up work; see #32362, #32369, #32372, #32378, #32381, and #32393. These references describe staged scope, not a claim that every follow-up has landed or every opset-28 operator is supported. Runtime function expansion is a full-build fallback, not minimal/ORT-format runtime coverage. ### Local validation Completed on Windows with retained CPU-only Ninja/MSVC `RelWithDebInfo` builds and strict released-opset policy: - Built `onnxruntime_provider_test`, `onnxruntime_test_all`, `onnx_test_runner`, and Python bindings against ONNX 1.23.2; retained build remains available for incremental work. - Source-built and installed the exact patched ONNX 1.23.2 Python wheel. - Separate `--minimal_build extended` shared runtime configured, compiled, and linked successfully with ML/contrib ops and unit tests disabled. This exercises `ONNX_MINIMAL_BUILD=ON`; it is not a minimal-runtime function-op execution test. - Mod/BitShift/SpaceToDepth/Cast provider tests: **247 passed**, no skips. SpaceToDepth checks default DCR, explicit DCR, and CRD with distinct literal expected outputs, expansion, and legacy opset boundaries. - Slice/Concat fusion and type-info/tensor-proto framework tests: **27 passed**, no skips. - Python backend tests: **22 passed**, plus **2 subtests passed**. - Full Python backend series: **4,066 tests**, **2,685 skipped**, zero failures. Skipped/unsupported coverage is not a pass. - Materialized **1,884 node cases**. Native CPU runner with `-m 1500` collected **1,760 cases**: **1,590 succeeded**, **170 not implemented**, **zero failed**, exit 0. These results do not establish complete opset-28 support. - Generated CPU operator documentation matches the rebuilt kernel registry; verified archive hashes, patch identity/application, submodule SHA, and changed-file lint. **Remaining validation/infrastructure limits:** GPU and other EP execution relies on CI; optional-provider routing checks are compiled into the relevant builds but were unavailable locally. Full unfiltered C++ suites and the ancillary broad `ALL` target were not rerun. The oracle-dependent corpus equivalence test skips because upstream no longer ships the corpus. The new ONNX 1.23.2 tarball currently returns **404 from the Microsoft vcpkg asset mirror**; it still needs Terrapin/infra seeding before read-only `x-block-origin` vcpkg legs can download it. FetchContent and the successful minimal build are mirror-independent. New-head CI is pending. --------- Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: gramalingam <10075881+gramalingam@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: gramalingam <10075881+gramalingam@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Ti-Tai Wang (titaiwangms)
left a comment
There was a problem hiding this comment.
I wonder if we need to trigger that python package run as well. Since we are bumping opset for some ops, there should be tests updated.
There was a problem hiding this comment.
🟡 Changes recommended
The new FP16 and BF16 Celu implementations silently replace NaN inputs with zero.
1 open finding
5 resolved since last review
This BF16 Einsum test will fail under CUDA builds because CUDA Einsum v28 is registered without… The BF16 ReduceSum adapter allocatesfloat_inputusinginput.Shape()even when… The accelerated BF16 MatMul path materializes full-size float buffers for all batches… The PR title/summary focuses on opset-28 BF16 enablement, but the generated docs update introduces… This build-time type guard always checksEnabledUniqueDataTypes28, even though the kernel is…
🧠 Review effort: Balanced
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Started: https://aiinfra.visualstudio.com/Lotus/_build/results?buildId=1481778&view=results |



Summary
Validation