Skip to content

[ARM] Remove UnsafeFPMath uses #151275

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.

Sign up for GitHub

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 5 commits into from
Sep 28, 2025
Merged

[ARM] Remove UnsafeFPMath uses #151275

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
59b2996
Remove UnsafeFPMath uses in ARM
paperchalice Jul 31, 2025
0dc3c99
use getDenormalModeRaw
paperchalice Aug 13, 2025
8cad2f8
address comments
paperchalice Aug 29, 2025
5a6a193
Use suggestion and skip declaration
paperchalice Sep 26, 2025
6be8d53
remove unsafe-fp-math related tests
paperchalice Sep 27, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 17 additions & 2 deletions llvm/lib/Target/ARM/ARMAsmPrinter.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -629,6 +629,19 @@ static bool checkDenormalAttributeConsistency(const Module &M,
});
}

// Returns true if all functions have different denormal modes.
static bool checkDenormalAttributeInconsistency(const Module &M) {
auto F = M.functions().begin();
auto E = M.functions().end();
if (F == E)
return false;
DenormalMode Value = F->getDenormalModeRaw();
++F;
return std::any_of(F, E, [&](const Function &F) {
return !F.isDeclaration() && F.getDenormalModeRaw() != Value;
});
}

void ARMAsmPrinter::emitAttributes() {
MCTargetStreamer &TS = *OutStreamer->getTargetStreamer();
ARMTargetStreamer &ATS = static_cast<ARMTargetStreamer &>(TS);
Expand Down Expand Up @@ -695,7 +708,9 @@ void ARMAsmPrinter::emitAttributes() {
DenormalMode::getPositiveZero()))
ATS.emitAttribute(ARMBuildAttrs::ABI_FP_denormal,
ARMBuildAttrs::PositiveZero);
else if (!TM.Options.UnsafeFPMath)
else if (checkDenormalAttributeInconsistency(*MMI->getModule()) ||
Copy link
Collaborator

@davemgreen davemgreen Sep 25, 2025

Choose a reason for hiding this comment

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

I don't think this is working very well, either before this patch or after it. Nothing we do will be perfect and the existing functions are already broken (I think they did better before 0ab5b5b).

I would have expected -ffast-math to set denormal attributes, if it makes use of crtfastmath.o. But that is not what happens at the moment.

Copy link
Contributor Author

@paperchalice paperchalice Sep 26, 2025

Choose a reason for hiding this comment

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

Can frontend emit some hints for this when targeting ARM?

Copy link
Collaborator

@davemgreen davemgreen Sep 27, 2025

Choose a reason for hiding this comment

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

That might be a sensible approach. We would need to make sure it worked well with LTO and whatnot. Lets try this first and see if anyone has problems with it. If so we can give the alternative a try. Thanks for your work on getting this done.

checkDenormalAttributeConsistency(
*MMI->getModule(), "denormal-fp-math", DenormalMode::getIEEE()))
ATS.emitAttribute(ARMBuildAttrs::ABI_FP_denormal,
ARMBuildAttrs::IEEEDenormals);
else {
Expand Down Expand Up @@ -730,7 +745,7 @@ void ARMAsmPrinter::emitAttributes() {
TM.Options.NoTrappingFPMath)
ATS.emitAttribute(ARMBuildAttrs::ABI_FP_exceptions,
ARMBuildAttrs::Not_Allowed);
else if (!TM.Options.UnsafeFPMath) {
else {
ATS.emitAttribute(ARMBuildAttrs::ABI_FP_exceptions, ARMBuildAttrs::Allowed);

// If the user has permitted this code to choose the IEEE 754
Expand Down
Loading