[clang] Lower modf builtin using `llvm.modf` intrinsic #126750

MacDue · 2025-02-11T15:50:33Z

This updates the existing modf[f|l] builtin to be lowered via the llvm.modf.* intrinsic (rather than directly to a library call).

This updates the existing `modf[f|l]` builtin to be lowered via the `llvm.modf.*` intrinsic when `-fno-math-errno` is set (rather than directly to a library call).

llvmbot · 2025-02-11T15:51:06Z

@llvm/pr-subscribers-backend-x86
@llvm/pr-subscribers-clang-codegen

@llvm/pr-subscribers-clang

Author: Benjamin Maxwell (MacDue)

Changes

This updates the existing modf[f|l] builtin to be lowered via the llvm.modf.* intrinsic when -fno-math-errno is set (rather than directly to a library call).

Full diff: https://github.com/llvm/llvm-project/pull/126750.diff

5 Files Affected:

(modified) clang/lib/CodeGen/CGBuiltin.cpp (+26)
(modified) clang/test/CodeGen/X86/math-builtins.c (+22-4)
(modified) clang/test/CodeGen/aix-builtin-mapping.c (+1-1)
(modified) clang/test/CodeGen/builtin-attributes.c (+1-1)
(modified) clang/test/CodeGen/math-libcalls.c (+6-6)

diff --git a/clang/lib/CodeGen/CGBuiltin.cpp b/clang/lib/CodeGen/CGBuiltin.cpp
index 361e4c4bf2e2ed..07522a62ccae6b 100644
--- a/clang/lib/CodeGen/CGBuiltin.cpp
+++ b/clang/lib/CodeGen/CGBuiltin.cpp
@@ -859,6 +859,24 @@ static void emitSincosBuiltin(CodeGenFunction &CGF, const CallExpr *E,
   StoreCos->setMetadata(LLVMContext::MD_noalias, AliasScopeList);
 }
 
+static llvm::Value *emitModfBuiltin(CodeGenFunction &CGF, const CallExpr *E,
+                                    llvm::Intrinsic::ID IntrinsicID) {
+  llvm::Value *Val = CGF.EmitScalarExpr(E->getArg(0));
+  llvm::Value *IntPartDest = CGF.EmitScalarExpr(E->getArg(1));
+
+  llvm::Function *F = CGF.CGM.getIntrinsic(IntrinsicID, {Val->getType()});
+  llvm::Value *Call = CGF.Builder.CreateCall(F, Val);
+
+  llvm::Value *FractionalResult = CGF.Builder.CreateExtractValue(Call, 0);
+  llvm::Value *IntegralResult = CGF.Builder.CreateExtractValue(Call, 1);
+
+  QualType DestPtrType = E->getArg(1)->getType()->getPointeeType();
+  LValue IntegralLV = CGF.MakeNaturalAlignAddrLValue(IntPartDest, DestPtrType);
+  CGF.Builder.CreateStore(IntegralResult, IntegralLV.getAddress());
+
+  return FractionalResult;
+}
+
 /// EmitFAbs - Emit a call to @llvm.fabs().
 static Value *EmitFAbs(CodeGenFunction &CGF, Value *V) {
   Function *F = CGF.CGM.getIntrinsic(Intrinsic::fabs, V->getType());
@@ -3259,6 +3277,14 @@ RValue CodeGenFunction::EmitBuiltinExpr(const GlobalDecl GD, unsigned BuiltinID,
       return RValue::get(Builder.CreateFRem(Arg1, Arg2, "fmod"));
     }
 
+    case Builtin::BImodf:
+    case Builtin::BImodff:
+    case Builtin::BImodfl:
+    case Builtin::BI__builtin_modf:
+    case Builtin::BI__builtin_modff:
+    case Builtin::BI__builtin_modfl:
+      return RValue::get(emitModfBuiltin(*this, E, Intrinsic::modf));
+
     case Builtin::BIlog:
     case Builtin::BIlogf:
     case Builtin::BIlogl:
diff --git a/clang/test/CodeGen/X86/math-builtins.c b/clang/test/CodeGen/X86/math-builtins.c
index d7bf7d57fba26d..08c7765dd4002d 100644
--- a/clang/test/CodeGen/X86/math-builtins.c
+++ b/clang/test/CodeGen/X86/math-builtins.c
@@ -38,6 +38,24 @@ void foo(double *d, float f, float *fp, long double *l, int *i, const char *c) {
 // NO__ERRNO-NEXT: [[FREXP_F128_0:%.+]] = extractvalue { fp128, i32 } [[FREXP_F128]], 0
 
 
+// NO__ERRNO: [[MODF_F64:%.+]] = call { double, double } @llvm.modf.f64(double %{{.+}})
+// NO__ERRNO-NEXT: [[MODF_F64_FP:%.+]] = extractvalue { double, double } [[MODF_F64]], 0
+// NO__ERRNO-NEXT: [[MODF_F64_IP:%.+]] = extractvalue { double, double } [[MODF_F64]], 1
+// NO__ERRNO-NEXT: store double [[MODF_F64_IP]], ptr %{{.+}}, align 8
+
+// NO__ERRNO: [[MODF_F32:%.+]] = call { float, float } @llvm.modf.f32(float %{{.+}})
+// NO__ERRNO-NEXT: [[MODF_F32_FP:%.+]] = extractvalue { float, float } [[MODF_F32]], 0
+// NO__ERRNO-NEXT: [[MODF_F32_IP:%.+]] = extractvalue { float, float } [[MODF_F32]], 1
+// NO__ERRNO-NEXT: store float [[MODF_F32_IP]], ptr %{{.+}}, align 4
+
+// NO__ERRNO: [[MODF_F80:%.+]] = call { x86_fp80, x86_fp80 } @llvm.modf.f80(x86_fp80 %{{.+}})
+// NO__ERRNO-NEXT: [[MODF_F80_FP:%.+]] = extractvalue { x86_fp80, x86_fp80 } [[MODF_F80]], 0
+// NO__ERRNO-NEXT: [[MODF_F80_IP:%.+]] = extractvalue { x86_fp80, x86_fp80 } [[MODF_F80]], 1
+// NO__ERRNO-NEXT: store x86_fp80 [[MODF_F80_IP]], ptr %{{.+}}, align 16
+
+// NO__ERRNO: call fp128 @modff128(fp128 noundef %{{.+}}, ptr noundef %{{.+}})
+
+
 // NO__ERRNO: [[SINCOS_F64:%.+]] = call { double, double } @llvm.sincos.f64(double %{{.+}})
 // NO__ERRNO-NEXT: [[SINCOS_F64_0:%.+]] = extractvalue { double, double } [[SINCOS_F64]], 0
 // NO__ERRNO-NEXT: [[SINCOS_F64_1:%.+]] = extractvalue { double, double } [[SINCOS_F64]], 1
@@ -139,10 +157,10 @@ void foo(double *d, float f, float *fp, long double *l, int *i, const char *c) {
 
   __builtin_modf(f,d);       __builtin_modff(f,fp);      __builtin_modfl(f,l); __builtin_modff128(f,l);
 
-// NO__ERRNO: declare double @modf(double noundef, ptr noundef) [[NOT_READNONE:#[0-9]+]]
-// NO__ERRNO: declare float @modff(float noundef, ptr noundef) [[NOT_READNONE]]
-// NO__ERRNO: declare x86_fp80 @modfl(x86_fp80 noundef, ptr noundef) [[NOT_READNONE]]
-// NO__ERRNO: declare fp128 @modff128(fp128 noundef, ptr noundef) [[NOT_READNONE]]
+// NO__ERRNO: declare { double, double } @llvm.modf.f64(double) [[READNONE_INTRINSIC]]
+// NO__ERRNO: declare { float, float } @llvm.modf.f32(float) [[READNONE_INTRINSIC]]
+// NO__ERRNO: declare { x86_fp80, x86_fp80 } @llvm.modf.f80(x86_fp80) [[READNONE_INTRINSIC]]
+// NO__ERRNO: declare fp128 @modff128(fp128 noundef, ptr noundef) [[NOT_READNONE:#[0-9]+]]
 // HAS_ERRNO: declare double @modf(double noundef, ptr noundef) [[NOT_READNONE]]
 // HAS_ERRNO: declare float @modff(float noundef, ptr noundef) [[NOT_READNONE]]
 // HAS_ERRNO: declare x86_fp80 @modfl(x86_fp80 noundef, ptr noundef) [[NOT_READNONE]]
diff --git a/clang/test/CodeGen/aix-builtin-mapping.c b/clang/test/CodeGen/aix-builtin-mapping.c
index a79218c6f1d8b9..cc1cc1a44f32ce 100644
--- a/clang/test/CodeGen/aix-builtin-mapping.c
+++ b/clang/test/CodeGen/aix-builtin-mapping.c
@@ -17,6 +17,6 @@ int main()
   returnValue = __builtin_ldexpl(1.0L, 1);
 }
 
-// CHECK: %call = call double @modf(double noundef 1.000000e+00, ptr noundef %DummyLongDouble) #3
+// CHECK: %{{.+}} = call { double, double } @llvm.modf.f64(double 1.000000e+00)
 // CHECK: %{{.+}} = call { double, i32 } @llvm.frexp.f64.i32(double 0.000000e+00)
 // CHECK: %{{.+}} = call double @llvm.ldexp.f64.i32(double 1.000000e+00, i32 1)
diff --git a/clang/test/CodeGen/builtin-attributes.c b/clang/test/CodeGen/builtin-attributes.c
index e5b0faccfd23f2..8461467b104698 100644
--- a/clang/test/CodeGen/builtin-attributes.c
+++ b/clang/test/CodeGen/builtin-attributes.c
@@ -1,5 +1,5 @@
 // REQUIRES: arm-registered-target
-// RUN: %clang_cc1 -triple arm-unknown-linux-gnueabi -emit-llvm -o - %s | FileCheck %s
+// RUN: %clang_cc1 -triple arm-unknown-linux-gnueabi -fmath-errno -emit-llvm -o - %s | FileCheck %s
 
 int printf(const char *, ...);
 void exit(int);
diff --git a/clang/test/CodeGen/math-libcalls.c b/clang/test/CodeGen/math-libcalls.c
index bcc61c8f046b43..507d3a9bd03692 100644
--- a/clang/test/CodeGen/math-libcalls.c
+++ b/clang/test/CodeGen/math-libcalls.c
@@ -83,15 +83,15 @@ void foo(double *d, float f, float *fp, long double *l, int *i, const char *c) {
 
   modf(f,d);       modff(f,fp);      modfl(f,l);
 
-  // NO__ERRNO: declare double @modf(double noundef, ptr noundef) [[NOT_READNONE]]
-  // NO__ERRNO: declare float @modff(float noundef, ptr noundef) [[NOT_READNONE]]
-  // NO__ERRNO: declare x86_fp80 @modfl(x86_fp80 noundef, ptr noundef) [[NOT_READNONE]]
+  // NO__ERRNO: declare { double, double } @llvm.modf.f64(double) [[READNONE_INTRINSIC]]
+  // NO__ERRNO: declare { float, float } @llvm.modf.f32(float) [[READNONE_INTRINSIC]]
+  // NO__ERRNO: declare { x86_fp80, x86_fp80 } @llvm.modf.f80(x86_fp80) [[READNONE_INTRINSIC]]
   // HAS_ERRNO: declare double @modf(double noundef, ptr noundef) [[NOT_READNONE]]
   // HAS_ERRNO: declare float @modff(float noundef, ptr noundef) [[NOT_READNONE]]
   // HAS_ERRNO: declare x86_fp80 @modfl(x86_fp80 noundef, ptr noundef) [[NOT_READNONE]]
-  // HAS_MAYTRAP: declare double @modf(double noundef, ptr noundef) [[NOT_READNONE]]
-  // HAS_MAYTRAP: declare float @modff(float noundef, ptr noundef) [[NOT_READNONE]]
-  // HAS_MAYTRAP: declare x86_fp80 @modfl(x86_fp80 noundef, ptr noundef) [[NOT_READNONE]]
+  // HAS_MAYTRAP: declare { double, double } @llvm.modf.f64(double) [[READNONE_INTRINSIC]]
+  // HAS_MAYTRAP: declare { float, float } @llvm.modf.f32(float) [[READNONE_INTRINSIC]]
+  // HAS_MAYTRAP: declare { x86_fp80, x86_fp80 } @llvm.modf.f80(x86_fp80) [[READNONE_INTRINSIC]]
 
   nan(c);        nanf(c);       nanl(c);

clang/lib/CodeGen/CGBuiltin.cpp

efriedma-quic · 2025-02-11T20:27:32Z

when -fno-math-errno is set

What does -fno-math-errno have to do with anything? modf never sets errno anyway.

clang/lib/CodeGen/CGBuiltin.cpp

MacDue · 2025-02-11T21:08:18Z

when -fno-math-errno is set

What does -fno-math-errno have to do with anything? modf never sets errno anyway.

With the current modf builtin (not changed here) it skips the code path for emitting intrinsics when math-errno is enabled. I think setting ConstIgnoringExceptions on the builtin would allow the intrinsics even with math-errno (though that's not necessary for our use case).

llvm-project/clang/lib/CodeGen/CGBuiltin.cpp

Lines 3024 to 3035 in 0121d99

    
           // Math intrinsics are generated only when math-errno is disabled. Any pragmas 
        
           // or attributes that affect math-errno should prevent or allow math 
        
           // intrincs to be generated. Intrinsics are generated: 
        
           //   1- In fast math mode, unless math-errno is overriden 
        
           //      via '#pragma float_control(precise, on)', or via an 
        
           //      'attribute__((optnone))'. 
        
           //   2- If math-errno was enabled on command line but overriden 
        
           //      to false via '#pragma float_control(precise, off))' and 
        
           //      'attribute__((optnone))' hasn't been used. 
        
           //   3- If we are compiling with optimization and errno has been disabled 
        
           //      via '#pragma float_control(precise, off)', and 
        
           //      'attribute__((optnone))' hasn't been used.

efriedma-quic · 2025-02-11T21:48:46Z

Can you just move the modf handling into the section for non-math intrinsics? It's not a "math intrinsic" in this context: it doesn't set errno or raise exceptions.

MacDue · 2025-02-12T13:30:58Z

Can you just move the modf handling into the section for non-math intrinsics? It's not a "math intrinsic" in this context: it doesn't set errno or raise exceptions.

Sure, done 👍

efriedma-quic

LGTM

efriedma-quic

Hang on... I need to think a little more about the interaction with snan in strictfp mode.

efriedma-quic · 2025-02-13T21:52:38Z

Referring to the C standard and implementations, I think modf does in fact raise an fp exception when the operand is an snan. So to handle this, we need different lowering in strictfp mode, probably to a "constrained" intrinsic.

MacDue · 2025-02-17T12:02:36Z

So to handle this, we need different lowering in strictfp mode, probably to a "constrained" intrinsic.

I've updated the lowering to simply bail out when Builder.getIsFPConstrained() set (which I think corresponds to strictfp). So now in the HAS_MAYTRAP case the lowering is unchanged.

clang/lib/CodeGen/CGBuiltin.cpp

vitalybuka · 2025-02-19T18:55:53Z

From this patch?
https://lab.llvm.org/buildbot/#/builders/72/builds/8380

FYI @fmayer

MacDue · 2025-02-19T20:22:06Z

From this patch? https://lab.llvm.org/buildbot/#/builders/72/builds/8380

FYI @fmayer

Looks like it, I had missed it in the log but the actual error is:

ExpandFloatResult #0: t57: ppcf128,ppcf128 = fmodf ConstantFP:ppcf128<APFloat(4611911198408756429)>, llvm-project/compiler-rt/lib/msan/tests/msan_test.cpp:1906:3
fatal error: error in backend: Do not know how to expand the result of this operator!

I'll see if I can prepare a fix.

MacDue · 2025-02-19T21:18:12Z

I think this patch should fix the issue: #127895

…)" This reverts commit d804c83.

) Reverts #126750 Revering while I investigate: https://lab.llvm.org/buildbot/#/builders/72/builds/8406

…nsic" (#127987) Reverts llvm/llvm-project#126750 Revering while I investigate: https://lab.llvm.org/buildbot/#/builders/72/builds/8406

dcandler · 2025-02-20T11:32:24Z

FYI, I was also a getting an error from this change, when trying to build picolibc for armv6m (as well as other arm targets):

SoftenFloatResult #0: t106: f64,f64 = fmodf t105, ../../../../../src/picolibc/test/math-funcs.c:85:10
fatal error: error in backend: Do not know how to soften the result of this operator!

The error is gone now the change has been reverted, but it's slightly different to the one reported above.

) Reverts #127987 Original description: This updates the existing modf[f|l] builtin to be lowered via the llvm.modf.* intrinsic (rather than directly to a library call). The legalization issues exposed by the original PR (#126750) should have been fixed in #128055 and #129264.

…c" (#129885)" This broke modff calls on 32-bit x86 Windows. See comment on the PR. > This updates the existing modf[f|l] builtin to be lowered via the > llvm.modf.* intrinsic (rather than directly to a library call). > > The legalization issues exposed by the original PR (#126750) should have > been fixed in #128055 and #129264. This reverts commit cd1d9a8.

…#129885) Reverts llvm#127987 Original description: This updates the existing modf[f|l] builtin to be lowered via the llvm.modf.* intrinsic (rather than directly to a library call). The legalization issues exposed by the original PR (llvm#126750) should have been fixed in llvm#128055 and llvm#129264.

[clang] Lower modf builtin using llvm.modf intrinsic

eee5bc8

This updates the existing `modf[f|l]` builtin to be lowered via the `llvm.modf.*` intrinsic when `-fno-math-errno` is set (rather than directly to a library call).

MacDue requested review from arsenm and sdesmalen-arm February 11, 2025 15:50

llvmbot added clang Clang issues not falling into any other category backend:X86 clang:codegen IR generation bugs: mangling, exceptions, etc. labels Feb 11, 2025

arsenm reviewed Feb 11, 2025

View reviewed changes

clang/lib/CodeGen/CGBuiltin.cpp Outdated Show resolved Hide resolved

Fixups

0121d99

efriedma-quic reviewed Feb 11, 2025

View reviewed changes

clang/lib/CodeGen/CGBuiltin.cpp Outdated Show resolved Hide resolved

Fixups

0c5a3a6

efriedma-quic approved these changes Feb 13, 2025

View reviewed changes

efriedma-quic requested changes Feb 13, 2025

View reviewed changes

Don't use intrinsic in constrained mode

7b1f5cc

arsenm reviewed Feb 17, 2025

View reviewed changes

clang/lib/CodeGen/CGBuiltin.cpp Show resolved Hide resolved

arsenm approved these changes Feb 17, 2025

View reviewed changes

clang/lib/CodeGen/CGBuiltin.cpp Show resolved Hide resolved

Add comment

6fa8a5b

MacDue requested a review from efriedma-quic February 18, 2025 06:50

efriedma-quic approved these changes Feb 18, 2025

View reviewed changes

MacDue merged commit d804c83 into llvm:main Feb 19, 2025
8 checks passed

MacDue deleted the modf_builtin branch February 19, 2025 14:49

MacDue mentioned this pull request Feb 19, 2025

[SDAG] Add missing ppc_fp128 ExpandFloatRes legalization for modf #127895

Merged

MacDue added a commit that referenced this pull request Feb 20, 2025

Revert "[clang] Lower modf builtin using llvm.modf intrinsic (#126750…

e082cbf

…)" This reverts commit d804c83.

MacDue mentioned this pull request Feb 20, 2025

Revert "[clang] Lower modf builtin using llvm.modf intrinsic" #127987

Merged

MacDue added a commit that referenced this pull request Feb 20, 2025

Revert "[clang] Lower modf builtin using llvm.modf intrinsic" (#127987

d595fc9

) Reverts #126750 Revering while I investigate: https://lab.llvm.org/buildbot/#/builders/72/builds/8406

joaosaffran mentioned this pull request Feb 26, 2025

[libc++] Set feature-test macro __cpp_lib_atomic_float (#127559) joaosaffran/llvm-project#1

Closed

MacDue mentioned this pull request Mar 5, 2025

Reland "[clang] Lower modf builtin using llvm.modf intrinsic" #129885

Merged

shiltian mentioned this pull request Mar 10, 2025

[AMDGPU] Fix test failures when expensive checks are enabled #130644

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[clang] Lower modf builtin using `llvm.modf` intrinsic #126750

[clang] Lower modf builtin using `llvm.modf` intrinsic #126750

MacDue commented Feb 11, 2025 •

edited

Loading

llvmbot commented Feb 11, 2025 •

edited

Loading

efriedma-quic commented Feb 11, 2025

MacDue commented Feb 11, 2025

efriedma-quic commented Feb 11, 2025

MacDue commented Feb 12, 2025

efriedma-quic left a comment

efriedma-quic left a comment

efriedma-quic commented Feb 13, 2025

MacDue commented Feb 17, 2025

vitalybuka commented Feb 19, 2025

MacDue commented Feb 19, 2025

MacDue commented Feb 19, 2025

dcandler commented Feb 20, 2025

[clang] Lower modf builtin using llvm.modf intrinsic #126750

[clang] Lower modf builtin using llvm.modf intrinsic #126750

Conversation

MacDue commented Feb 11, 2025 • edited Loading

llvmbot commented Feb 11, 2025 • edited Loading

efriedma-quic commented Feb 11, 2025

MacDue commented Feb 11, 2025

efriedma-quic commented Feb 11, 2025

MacDue commented Feb 12, 2025

efriedma-quic left a comment

Choose a reason for hiding this comment

efriedma-quic left a comment

Choose a reason for hiding this comment

efriedma-quic commented Feb 13, 2025

MacDue commented Feb 17, 2025

vitalybuka commented Feb 19, 2025

MacDue commented Feb 19, 2025

MacDue commented Feb 19, 2025

dcandler commented Feb 20, 2025

[clang] Lower modf builtin using `llvm.modf` intrinsic #126750

[clang] Lower modf builtin using `llvm.modf` intrinsic #126750

MacDue commented Feb 11, 2025 •

edited

Loading

llvmbot commented Feb 11, 2025 •

edited

Loading