Skip to content

[BugFix] Align default (C) tirx.round lowering to ties-to-even - #20131

Open
LngelKyo wants to merge 2 commits into
apache:mainfrom
LngelKyo:fix/c-round-ties-to-even
Open

[BugFix] Align default (C) tirx.round lowering to ties-to-even#20131
LngelKyo wants to merge 2 commits into
apache:mainfrom
LngelKyo:fix/c-round-ties-to-even

Conversation

@LngelKyo

Copy link
Copy Markdown

#19368 aligned tir.round to ties-to-even "across all backends" and moved LLVM, CUDA, NVPTX, ROCm, Hexagon, Metal, OpenCL, SPIR-V and WebGPU. It did not touch src/target/intrin_rule.cc, so the default.FLowerIntrinsic rule — which the c target uses — still lowers through FloatSuffix and emits round/roundf, i.e. ties away from zero.

This disagrees with:

Why CI did not catch it. test_target_codegen_c_host.py::test_round compiles with target="c" and asserts against np.round, which is ties-to-even — so the intended semantics was already encoded. It passes only because its input is np.random.rand, which does not produce exact midpoints (0 occurrences in 2,048,000 sampled values). The ties-to-even test added by #19368 (test_tir_intrin.py::test_round_ties_to_even) only runs target="llvm", so no test covered the C path's tie behaviour.

The first commit adds the midpoint vector — reused verbatim from test_round_ties_to_even — to the existing C-host test and it fails:

E   Mismatch at indices:
E    [0]: 1.0 (ACTUAL), 0.0 (DESIRED)
E    [2]: 3.0 (ACTUAL), 2.0 (DESIRED)
E    [4]: -1.0 (ACTUAL), -0.0 (DESIRED)
E    [6]: -3.0 (ACTUAL), -2.0 (DESIRED)

The second commit renames round→nearbyint before the float suffix, mirroring the existing CUDA rule, and it passes.

One note for reviewers: nearbyint honours the current floating-point environment and is ties-to-even under the default FE_TONEAREST. That is the same guarantee LLVM, CUDA, Metal and OpenCL already rely on after #19368; this PR does not introduce a new assumption.

LngelKyo added 2 commits August 16, 2026 00:58
np.random.rand never produces exact midpoints, so the existing input
cannot distinguish ties-to-even (np.round, constant folding, and every
other backend) from ties-away-from-zero (C round/roundf). Reuse the
midpoint vector from test_round_ties_to_even verbatim. This test fails
against current main.
tirx.round is ties-to-even, and constant folding implements it with
std::nearbyint, but the default.FLowerIntrinsic rule lowered it through
FloatSuffix to the C library round()/roundf(), which is ties away from
zero. Rename to nearbyint before the float suffix is applied, mirroring
the existing CUDA rule. Fixes the C target's disagreement with the
constant folder and with every other backend after apache#19368.
@LngelKyo

Copy link
Copy Markdown
Author

@swjng @tlopex — cc, since this completes the backend sweep from #19368.

CI has not been triggered on this PR (no check runs); I believe it needs approval as this is my first contribution here. Happy to rebase or adjust if anything is needed.

@yongwww yongwww closed this Aug 17, 2026
@yongwww yongwww reopened this Aug 17, 2026
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.

2 participants