Skip to content
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

Shared library compiled with -ffast-math modifies FPU state #57589

Closed
simonbyrne opened this issue Sep 6, 2022 · 9 comments · Fixed by #80475
Closed

Shared library compiled with -ffast-math modifies FPU state #57589

simonbyrne opened this issue Sep 6, 2022 · 9 comments · Fixed by #80475
Labels
clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl' floating-point Floating-point math

Comments

@simonbyrne
Copy link

simonbyrne commented Sep 6, 2022

On linux x86_64

touch foo.c
clang -Ofast -fpic -shared foo.c -o foo.so
objdump --disassemble foo.so

gives:

Disassembly of section .text:

00000000000004f0 <set_fast_math>:
 4f0:	0f ae 5c 24 fc       	stmxcsr -0x4(%rsp)
 4f5:	81 4c 24 fc 40 80 00 	orl    $0x8040,-0x4(%rsp)
 4fc:	00
 4fd:	0f ae 54 24 fc       	ldmxcsr -0x4(%rsp)
 502:	c3                   	retq
 503:	66 2e 0f 1f 84 00 00 	nopw   %cs:0x0(%rax,%rax,1)
 50a:	00 00 00
 50d:	0f 1f 00             	nopl   (%rax)

This means that any thread which later loads the library will then set the flush subnormals to zero (FTZ) and subnormals are zero (DAZ) flags (even if the executable itself was not compiled with -ffast-math).

Related discussion

@simonbyrne simonbyrne changed the title Shared library compiled with -ffast-math modifies FPU state of thread Shared library compiled with -ffast-math modifies FPU state Sep 6, 2022
@jyknight
Copy link
Member

jyknight commented Sep 6, 2022

I wish nobody would ever use -Ofast or -ffast-math (as you say, "friends don't let friends use fast-math"). That said, this issue does seem like even more of a misfeature even than the other issues caused by fast-math.

However, currently, Clang will can link against a crtfastmath.o if one is present, but it doesn't actually ship one itself. This behavior will only occur if you have a system GCC installation.

So, since this behavior is effectively just for GCC compatibility, I think Clang probably ought to follow GCC's lead here -- that is, if any changes are made for GCC's 55522, then implement a parallel change to Clang, otherwise leave it as is.

@moyix
Copy link

moyix commented Sep 6, 2022

The gcc bug is almost 10 years old at this point with no sign of movement, and this particular behavior is continuing to cause problems in projects, many of which don't realize that they're enabling something with global effects. As a harm reduction measure, it would be really great to at least avoid linking in crtfastmath when building a shared library (even if present)?

@simonbyrne
Copy link
Author

simonbyrne commented Sep 6, 2022

However, currently, Clang will can link against a crtfastmath.o if one is present, but it doesn't actually ship one itself. This behavior will only occur if you have a system GCC installation.

Somehow this seems even worse.

@jyknight
Copy link
Member

jyknight commented Sep 6, 2022

I agree this is an unfortunate behavior.

Yet, I think it would also be poor to diverge Clang's behavior from GCC's here. It wouldn't be particularly helpful from a practical standpoint, since I expect these python packages are generally built with GCC when targeting linux anyhow.

Note that the implementation in clang, to match GCC's behavior, was explicitly requested by a user in #14396 a decade back.

@simonbyrne
Copy link
Author

simonbyrne commented Sep 6, 2022

If you don't want to remove it, then would it be possible to:

  • have an option to specifically disable crtfastmath
  • make it a warning to use crtfastmath with -shared
  • document it

@kiufta
Copy link

kiufta commented Sep 7, 2022

However, currently, Clang will can link against a crtfastmath.o if one is present, but it doesn't actually ship one itself. This behavior will only occur if you have a system GCC installation.

I think there are enough safeguards. We can't protect every schlub from incorrect use.

Only schlubs vote this post down ;)

@jcranmer-intel jcranmer-intel added the clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl' label Sep 8, 2022
@llvmbot
Copy link
Collaborator

llvmbot commented Sep 8, 2022

@llvm/issue-subscribers-clang-driver

@jcranmer-intel
Copy link
Contributor

I agree this is an unfortunate behavior.

Yet, I think it would also be poor to diverge Clang's behavior from GCC's here. It wouldn't be particularly helpful from a practical standpoint, since I expect these python packages are generally built with GCC when targeting linux anyhow.

Note that the implementation in clang, to match GCC's behavior, was explicitly requested by a user in #14396 a decade back.

This wouldn't be the first case where appealing to GCC's behavior leads to outcome that virtually everyone agrees is problematic. (oh hi -fp-contract and #pragma STDC FP_CONTRACT).

We definitely need to at least document this behavior so that people are aware what the consequences of using -ffast-math are. I strongly suspect that very few users are aware that modules compiled with -ffast-math can have effects on modules not compiled with -ffast-math.

Actually, the behavior with crtfastmath.o is even worse than it seems! We have -ffp-denormal[32]= behavior that would seem to suggest how we are handling subnormals, but setting -ffp-denormal=ieee doesn't disable linking with crtfastmath.o, since that logic only looks for -f[no-]fast-math and -f[no-]unsafe-math-optimizations (unless you compile with -Ofast, at which point crtfastmath.o is unconditionally linked because GCC!).

@jyknight
Copy link
Member

jyknight commented Jan 9, 2023

GCC has now changed the behavior in https://gcc.gnu.org/PR55522 to no longer link crtfastmath.o to -ffast-math -shared builds by default, and has also added a new -mdaz-ftz -mno-daz-ftz option to explicitly override and enable or disable linking of crtfastmath.o explicitly.

DaMatrix added a commit to PorkStudios/FarPlaneTwo that referenced this issue Jul 16, 2023
see https://gcc.gnu.org/bugzilla/show_bug.cgi?id=55522 and llvm/llvm-project#57589 for a summary. the FPU flags which are set as a result of linking with -ffast-math cause ALL denormals to be rounded to zero across the entire program. among other things, this causes java's Double.parseDouble to enter an infinite loop when attempting to parse denormal values (e.g. Ancient Warfare 2 would hang during startup while attempting to call Double.parseDouble with a string value of "1.582162261062063E-308").
@arsenm arsenm added the floating-point Floating-point math label Aug 14, 2023
DaMatrix added a commit to PorkStudios/FarPlaneTwo that referenced this issue Dec 17, 2023
see https://gcc.gnu.org/bugzilla/show_bug.cgi?id=55522 and llvm/llvm-project#57589 for a summary. the FPU flags which are set as a result of linking with -ffast-math cause ALL denormals to be rounded to zero across the entire program. among other things, this causes java's Double.parseDouble to enter an infinite loop when attempting to parse denormal values (e.g. Ancient Warfare 2 would hang during startup while attempting to call Double.parseDouble with a string value of "1.582162261062063E-308").

(cherry picked from commit 29ada41)

# Conflicts:
#	src/main/native/project.mk
jcranmer-intel added a commit to jcranmer-intel/llvm-project that referenced this issue Feb 22, 2024
This fixes llvm#57589, and aligns
Clang with the behavior of current versions of gcc. There is a new option,
-mdaz-ftz, to control the linking of the file that sets FTZ/DAZ on startup, and
this flag is on by default if -ffast-math is present and -shared isn't.
jcranmer-intel added a commit that referenced this issue Apr 25, 2024
This fixes #57589, and aligns
Clang with the behavior of current versions of gcc. There is a new
option, -mdaz-ftz, to control the linking of the file that sets FTZ/DAZ
on startup, and this flag is on by default if -ffast-math is present and
-shared isn't.

This also partially reverts fa7cd54 in that it disables the attempt
to set the IR denormal-fp-math attribute based on whether or not
-ffast-math is applied as it is insufficiently reliable.
thexujie pushed a commit to thexujie/llvm-project that referenced this issue May 7, 2024
This fixes llvm/llvm-project#57589, and aligns
Clang with the behavior of current versions of gcc. There is a new option,
-mdaz-ftz, to control the linking of the file that sets FTZ/DAZ on startup, and
this flag is on by default if -ffast-math is present and -shared isn't.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl' floating-point Floating-point math
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants