Skip to content

libc: Prevent FCSEL instruction from being used to avoid raising an unintended exception #24185

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jun 15, 2025

Conversation

papparapa
Copy link
Contributor

closes #24184

According to the comment, this line of the current sin function evaluates x + 0x1p120 and raises an INEXACT exception if x is not zero.
It also evaluates x / 0x1p120 and raises an UNDERFLOW exception if x is a non-normalized number.

// raise inexact if x != 0 and underflow if subnormal
if (common.want_float_exceptions) mem.doNotOptimizeAway(if (ix < 0x00100000) x / 0x1p120 else x + 0x1p120);

This is the disassembly results of the sin function test built in #24184.

$ llvm-objdump -S -l -d ./zig-out/bin/sin
[omitted]
; /workspaces/zig/lib/compiler_rt/sin.zig:101
;             if (common.want_float_exceptions) mem.doNotOptimizeAway(if (ix < 0x00100000) x / 0x1p120 else x + 0x1p120);
 1075300: f144011f     	cmp	x8, #0x100, lsl #12     // =0x100000
 1075304: 9e670121     	fmov	d1, x9
 1075308: 9e670142     	fmov	d2, x10
 107530c: 910023e8     	add	x8, sp, #0x8
 1075310: 1e610801     	fmul	d1, d0, d1
 1075314: 1e622802     	fadd	d2, d0, d2
 1075318: 1e623c21     	fcsel	d1, d1, d2, lo
[omitted]

FCSEL instruction selects one of the two registers according to the condition and copies its value.
The FCSEL instruction is preceded by the FMUL and FADD instructions, indicating that both x / 0x1p120 and x + 0x1p120 are evaluated.
It selects one or the other based on the result of ix < 0x00100000, but if a floating point exception occurs in the calculation of the one not selected, the exception is reflected in the final result.
In other words, it is effective in reducing branching, but both the FMUL and FADD instructions can have the side effect of raising a floating point exception, which may cause an exception that the programmer does not intend.

To prevent FCSEL instruction from being used here, this PR splits doNotOptimizeAway in two.
This change adds a branch instruction, but since this code path only passes if x is very small (|x| < 2^-26 in sin), I believe this change is acceptable.

In fact, after this PR, unintended exceptions no longer occur.

$ ./zig-out/bin/sin
$

Several other functions are similarly changed to prevent from raising unintended exceptions.

  • sin
  • sinf
  • sincos
  • sincosf
  • tan
  • tanf

…nintended exception

If you write an if expression in mem.doNotOptimizeAway like
doNotOptimizeAway(if (ix < 0x00100000) x / 0x1p120 else x + 0x1p120);,
FCSEL instruction is used on AArch64.
FCSEL instruction selects one of the two registers according to
the condition and copies its value.
In this example, `x / 0x1p120` and `x + 0x1p120` are expressions
that raise different floating-point exceptions.
However, since both are actually evaluated before the FCSEL
instruction, the exception not intended by the programmer may
also be raised.

To prevent FCSEL instruction from being used here, this commit
splits doNotOptimizeAway in two.
Copy link
Member

@jacobly0 jacobly0 left a comment

Choose a reason for hiding this comment

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

Looks reasonable.

@jacobly0 jacobly0 merged commit 878b7b8 into ziglang:master Jun 15, 2025
10 checks passed
@papparapa papparapa deleted the split-doNotOptimizeAway branch June 15, 2025 08:29
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.

On AArch64 sin function causes floating point exception different from musl
2 participants