Skip to content

[BugFix][ONNX] Fix Round op to use ties-to-even#19367

Merged
tlopex merged 1 commit into
apache:mainfrom
swjng:fix/onnx-round-ties-to-even
Apr 7, 2026
Merged

[BugFix][ONNX] Fix Round op to use ties-to-even#19367
tlopex merged 1 commit into
apache:mainfrom
swjng:fix/onnx-round-ties-to-even

Conversation

@swjng

@swjng swjng commented Apr 7, 2026

Copy link
Copy Markdown
Contributor

Problem

The ONNX Round operator specification requires ties-to-even (banker's) rounding:

"For cases where number is exactly halfway between two integers, it rounds to the nearest even integer."
https://onnx.ai/onnx/operators/onnx__Round.html

However, the current TVM implementation produces ties-away-from-zero results on midpoint values:

Input Expected (ties-to-even) Actual (ties-away)
0.5 0.0 1.0
1.5 2.0 2.0
2.5 2.0 3.0
-0.5 0.0 -1.0
-2.5 -2.0 -3.0

This was reported in issue #18590.

Root Cause

The lowering chain for relax.op.round:

relax.op.round -> (LegalizeOps) -> topi.round() -> te.round -> tir.round -> llvm::round

llvm::round is defined as ties-away-from-zero (C99 round()), while llvm::nearbyint uses the IEEE 754 default rounding mode (ties-to-even).

Fix

python/tvm/topi/math.py: Switch topi.round() from te.round to te.nearbyint. This lowers to tir.nearbyint -> llvm::nearbyint, which respects IEEE 754 ties-to-even.

src/target/source/intrin_rule_webgpu.cc: Register tir.nearbyint for the WebGPU backend. WGSL round() is already ties-to-even per the WGSL spec, so tir.nearbyint -> round is the correct mapping.

tests/python/relax/test_frontend_onnx.py: Add test_round_ties_to_even() with explicit midpoint inputs to prevent regression.

Testing

python -m pytest tests/python/relax/test_frontend_onnx.py::test_round_ties_to_even -xvs
python -m pytest "tests/python/relax/test_frontend_onnx.py::test_unary[Round]" -xvs

Both pass. The new test compares TVM output against onnxruntime (which correctly implements ties-to-even) for inputs [0.5, 1.5, 2.5, -0.5, -1.5, -2.5].

Fixes #18590

The ONNX Round operator specification requires ties-to-even rounding:
"For cases where number is exactly halfway between two integers, it
rounds to the nearest even integer."

Previously, `topi.round()` lowered to `te.round` -> `tir.round` ->
`llvm::round`, which uses ties-away-from-zero. This caused TVM to
return wrong results for midpoint values (e.g., round(0.5) = 1
instead of 0, round(2.5) = 3 instead of 2).

Fix by switching `topi.round()` to `te.nearbyint`, which lowers to
`tir.nearbyint` -> `llvm::nearbyint`. The `nearbyint` intrinsic
respects the IEEE 754 default rounding mode (ties-to-even), matching
the ONNX spec and onnxruntime behavior.

Also register `tir.nearbyint` for the WebGPU backend, mapping to
WGSL `round()` which is already ties-to-even per the WGSL spec.

Add a targeted test with midpoint inputs to prevent regression.

Fixes apache#18590

@gemini-code-assist gemini-code-assist Bot 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.

Code Review

This pull request updates the round operation in TVM TOPI to use ties-to-even (banker's rounding) by switching the underlying implementation to te.nearbyint, ensuring alignment with ONNX and IEEE 754 standards. It also adds the necessary intrinsic registration for WebGPU and a new test case to verify rounding behavior. Feedback indicates that the existing tirx.round registration on WebGPU is now inconsistent because it also maps to the ties-to-even round function, whereas it typically implies ties-away-from-zero; a follow-up is suggested to address this discrepancy.

Comment thread src/target/source/intrin_rule_webgpu.cc
@swjng

swjng commented Apr 7, 2026

Copy link
Copy Markdown
Contributor Author

As noted in the comment above, the same semantics is intentional. The actual pre-existing inconsistency runs in the opposite direction: LLVM/ROCm/Hexagon backends lower tirx.round to llvm::round (ties-away), contradicting the constant-folding behavior and op.h documentation. This affects LLVM, CUDA, ROCm, Hexagon, and potentially Metal/OpenCL backends.

Worth a follow-up issue to align all backends — tirx.roundllvm::nearbyint for LLVM-based targets would be the straightforward fix, though Metal/OpenCL need separate investigation.

@swjng swjng changed the title [BugFix][ONNX] Fix Round op to use ties-to-even (banker's rounding) [BugFix][ONNX] Fix Round op to use ties-to-even Apr 7, 2026

@tlopex tlopex left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

@tlopex

tlopex commented Apr 7, 2026

Copy link
Copy Markdown
Member

@swjng Thanks for the contribution! Could you send a follow-up PR to fix the problem you said?

@tlopex tlopex merged commit acfc483 into apache:main Apr 7, 2026
12 checks passed
@swjng

swjng commented Apr 8, 2026

Copy link
Copy Markdown
Contributor Author

@tlopex Yes, will make a follow-up

swjng added a commit to swjng/tvm that referenced this pull request Apr 8, 2026
tir.round constant-folds using std::nearbyint (ties-to-even), but
backends lowered it to platform round() (ties-away-from-zero). This
inconsistency meant compiled code could produce different results from
constant-folded code for midpoint values like 0.5, 2.5, etc.

Fix all backends to use ties-to-even intrinsics:
- LLVM/ROCm/Hexagon: llvm::Intrinsic::round -> nearbyint
- NVPTX: __nv_round -> __nv_nearbyint
- CUDA: round/roundf -> nearbyint/nearbyintf (f16/bf16 already used hrint)
- Metal/OpenCL: round -> rint
- Vulkan/SPIR-V: GLSLstd450Round -> GLSLstd450RoundEven
- OpenCL codegen: fix nearbyint mapping from round() to rint()

Also update op.h documentation to explicitly state ties-to-even semantics
and add test_round_ties_to_even regression test.

Follow-up to apache#19367.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
swjng added a commit to swjng/tvm that referenced this pull request Apr 8, 2026
tir.round constant-folds using std::nearbyint (ties-to-even), but
backends lowered it to platform round() (ties-away-from-zero). This
inconsistency meant compiled code could produce different results from
constant-folded code for midpoint values like 0.5, 2.5, etc.

Fix all backends to use ties-to-even intrinsics:
- LLVM/ROCm/Hexagon: llvm::Intrinsic::round -> nearbyint
- NVPTX: __nv_round -> __nv_nearbyint
- CUDA: round/roundf -> nearbyint/nearbyintf (f16/bf16 already used hrint)
- Metal/OpenCL: round -> rint
- Vulkan/SPIR-V: GLSLstd450Round -> GLSLstd450RoundEven
- OpenCL codegen: fix nearbyint mapping from round() to rint()

Also update op.h documentation to explicitly state ties-to-even semantics
and add test_round_ties_to_even regression test.

Follow-up to apache#19367.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
tlopex pushed a commit that referenced this pull request Apr 8, 2026
## Problem

`tir.round` constant-folds using `std::nearbyint` (IEEE 754
ties-to-even), but all backends lower it to platform `round()` which
uses ties-away-from-zero. This means compiled code can produce different
results from constant-folded code for midpoint values:

| Input | Constant-fold (ties-to-even) | Compiled (ties-away) |
|-------|-----|------|
| 0.5   | 0.0 | 1.0  |
| 2.5   | 2.0 | 3.0  |
| -0.5  | 0.0 | -1.0 |

This was identified as a follow-up to #19367 — see [this
comment](#19367 (comment)).

## Fix

Align all backends to use ties-to-even intrinsics, matching the
constant-folding behavior:

| Backend | Before | After |
|---------|--------|-------|
| LLVM/ROCm/Hexagon | `llvm::Intrinsic::round` |
`llvm::Intrinsic::nearbyint` |
| NVPTX | `__nv_round[f]` | `__nv_nearbyint[f]` |
| CUDA | `round`/`roundf` | `nearbyint`/`nearbyintf` (f16/bf16 already
used `hrint`) |
| Metal/OpenCL | `round` | `rint` |
| Vulkan/SPIR-V | `GLSLstd450Round` | `GLSLstd450RoundEven` |

Also fixes OpenCL codegen where `tir.nearbyint` was incorrectly mapped
to OpenCL `round()` instead of `rint()`.

Updates `op.h` documentation to explicitly state ties-to-even semantics
for both `round()` and `nearbyint()`.

## Testing

```
python -m pytest tests/python/tirx-base/test_tir_intrin.py -xvs
```

New `test_round_ties_to_even` verifies midpoint inputs `[0.5, 1.5, 2.5,
3.5, -0.5, -1.5, -2.5, -3.5]` produce ties-to-even results on the LLVM
backend. All 12 tests pass (10 passed, 2 skipped for CUDA).

---------

Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

2 participants