Page MenuHomePhabricator

Please use GitHub pull requests for new patches. Avoid migrating existing patches. Phabricator shutdown timeline

Disable call to fma for soft-float
ClosedPublic

Authored by long5hot on Jul 26 2023, 9:37 AM.

Details

Summary

PowerPC backend generate calls to libc function calls
for soft-float, regardless of the -nostdlib /-ffreestanding flag.
fma is not a function provided by compiler-rt builtins and
thus should not be generated here.
PR : #55230

Below is patch given by @nemanjai

Diff Detail

Event Timeline

long5hot created this revision.Jul 26 2023, 9:37 AM
Herald added a project: Restricted Project. · View Herald TranscriptJul 26 2023, 9:37 AM
long5hot requested review of this revision.Jul 26 2023, 9:37 AM
Herald added a project: Restricted Project. · View Herald TranscriptJul 26 2023, 9:37 AM
long5hot edited the summary of this revision. (Show Details)Jul 26 2023, 9:40 AM
long5hot updated this revision to Diff 544432.Jul 26 2023, 10:40 AM
This comment was removed by long5hot.
Herald added a project: Restricted Project. · View Herald TranscriptJul 26 2023, 10:40 AM
long5hot updated this revision to Diff 544437.Jul 26 2023, 10:46 AM
long5hot updated this revision to Diff 544438.Jul 26 2023, 10:48 AM

Apologies for updating diff too many times, used arcanist first time. Won't happend again!

compiler-rt builtins library should not just undefining fma for PPC, right (see https://gcc.gnu.org/onlinedocs/gccint/Soft-float-library-routines.html)? If so, maybe we should at least first try not generating fma for soft-float at the first place where fmul + fadd is fused, like tryEmitFMulAdd in clang front end?

compiler-rt builtins library should not just undefining fma for PPC, right (see https://gcc.gnu.org/onlinedocs/gccint/Soft-float-library-routines.html)? If so, maybe we should at least first try not generating fma for soft-float at the first place where fmul + fadd is fused, like tryEmitFMulAdd in clang front end?

I actually raised D154605 for that one, but it's only for X86.
I was relying on target-features containing "+fma", which is wrong. The review was raised just to get feedback from community working on various targets.
Plan is to disable fma-intrinsic from IR for all targets which doesn't support fma.

compiler-rt builtins library should not just undefining fma for PPC, right (see https://gcc.gnu.org/onlinedocs/gccint/Soft-float-library-routines.html)? If so, maybe we should at least first try not generating fma for soft-float at the first place where fmul + fadd is fused, like tryEmitFMulAdd in clang front end?

I actually raised D154605 for that one, but it's only for X86.
I was relying on target-features containing "+fma", which is wrong. The review was raised just to get feedback from community working on various targets.
Plan is to disable fma-intrinsic from IR for all targets which doesn't support fma.

Thanks. I think soft-float looks like a candidate for the hasFMA function in D154605.

jhibbits accepted this revision.Aug 23 2023, 1:29 PM
This revision is now accepted and ready to land.Aug 23 2023, 1:29 PM
long5hot added a comment.EditedAug 24 2023, 1:54 AM

@nemanjai , @jhibbits should i land on this commit, because as @shchenz said, similiar to D158632 review we are having plan to do same for PPC.

landing on this PR, from the conclusion of #55230 and D158632 conversation..
"If the llvm.fmuladd intrinsic reaches the backend, the backend is responsible for deciding how to represent it."

This revision was landed with ongoing or failed builds.Sep 28 2023, 1:37 AM
This revision was automatically updated to reflect the committed changes.