Skip to content

Conversation

wzssyqa
Copy link
Contributor

@wzssyqa wzssyqa commented Jun 21, 2024

We just introduce llvm.minimumnum and llvm.maximumnum intrinsics support to llvm. Let's support them in Clang.

See: #93033

@llvmbot llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" clang:codegen IR generation bugs: mangling, exceptions, etc. labels Jun 21, 2024
@llvmbot
Copy link
Member

llvmbot commented Jun 21, 2024

@llvm/pr-subscribers-clang-codegen

@llvm/pr-subscribers-clang

Author: YunQiang Su (wzssyqa)

Changes

We just introduce llvm.minimumnum and llvm.maximumnum intrinsics support to llvm. Let's support them in Clang.

See: #93033


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

6 Files Affected:

  • (modified) clang/include/clang/Basic/Builtins.td (+28)
  • (modified) clang/lib/CodeGen/CGBuiltin.cpp (+24)
  • (modified) clang/lib/Tooling/Inclusions/Stdlib/CSymbolMap.inc (+6)
  • (modified) clang/lib/Tooling/Inclusions/Stdlib/StdSymbolMap.inc (+18)
  • (modified) clang/test/CodeGen/builtins.c (+18)
  • (modified) clang/test/CodeGen/math-libcalls.c (+25)
diff --git a/clang/include/clang/Basic/Builtins.td b/clang/include/clang/Basic/Builtins.td
index 9342b6bc75fc8..d8093a7772b65 100644
--- a/clang/include/clang/Basic/Builtins.td
+++ b/clang/include/clang/Basic/Builtins.td
@@ -215,6 +215,18 @@ def FminF16F128 : Builtin, F16F128MathTemplate {
   let Prototype = "T(T, T)";
 }
 
+def FmaximumNumF16F128 : Builtin, F16F128MathTemplate {
+  let Spellings = ["__builtin_fmaximum_num"];
+  let Attributes = [FunctionWithBuiltinPrefix, NoThrow, Const, Constexpr];
+  let Prototype = "T(T, T)";
+}
+
+def FminimumNumF16F128 : Builtin, F16F128MathTemplate {
+  let Spellings = ["__builtin_fminimum_num"];
+  let Attributes = [FunctionWithBuiltinPrefix, NoThrow, Const, Constexpr];
+  let Prototype = "T(T, T)";
+}
+
 def Atan2F128 : Builtin {
   let Spellings = ["__builtin_atan2f128"];
   let Attributes = [FunctionWithBuiltinPrefix, NoThrow, ConstIgnoringErrnoAndExceptions];
@@ -3636,6 +3648,22 @@ def Fmin : FPMathTemplate, LibBuiltin<"math.h"> {
   let OnlyBuiltinPrefixedAliasIsConstexpr = 1;
 }
 
+def FmaximumNum : FPMathTemplate, LibBuiltin<"math.h"> {
+  let Spellings = ["fmaximum_num"];
+  let Attributes = [NoThrow, Const];
+  let Prototype = "T(T, T)";
+  let AddBuiltinPrefixedAlias = 1;
+  let OnlyBuiltinPrefixedAliasIsConstexpr = 1;
+}
+
+def FminimumNum : FPMathTemplate, LibBuiltin<"math.h"> {
+  let Spellings = ["fminimum_num"];
+  let Attributes = [NoThrow, Const];
+  let Prototype = "T(T, T)";
+  let AddBuiltinPrefixedAlias = 1;
+  let OnlyBuiltinPrefixedAliasIsConstexpr = 1;
+}
+
 def Hypot : FPMathTemplate, LibBuiltin<"math.h"> {
   let Spellings = ["hypot"];
   let Attributes = [NoThrow, ConstIgnoringErrnoAndExceptions];
diff --git a/clang/lib/CodeGen/CGBuiltin.cpp b/clang/lib/CodeGen/CGBuiltin.cpp
index 2516ed4508242..a9f2245305ec2 100644
--- a/clang/lib/CodeGen/CGBuiltin.cpp
+++ b/clang/lib/CodeGen/CGBuiltin.cpp
@@ -2794,6 +2794,30 @@ RValue CodeGenFunction::EmitBuiltinExpr(const GlobalDecl GD, unsigned BuiltinID,
                                    Intrinsic::minnum,
                                    Intrinsic::experimental_constrained_minnum));
 
+    case Builtin::BIfmaximum_num:
+    case Builtin::BIfmaximum_numf:
+    case Builtin::BIfmaximum_numl:
+    case Builtin::BI__builtin_fmaximum_num:
+    case Builtin::BI__builtin_fmaximum_numf:
+    case Builtin::BI__builtin_fmaximum_numf16:
+    case Builtin::BI__builtin_fmaximum_numl:
+    case Builtin::BI__builtin_fmaximum_numf128:
+      return RValue::get(emitBinaryMaybeConstrainedFPBuiltin(
+          *this, E, Intrinsic::maximumnum,
+          Intrinsic::experimental_constrained_maximumnum));
+
+    case Builtin::BIfminimum_num:
+    case Builtin::BIfminimum_numf:
+    case Builtin::BIfminimum_numl:
+    case Builtin::BI__builtin_fminimum_num:
+    case Builtin::BI__builtin_fminimum_numf:
+    case Builtin::BI__builtin_fminimum_numf16:
+    case Builtin::BI__builtin_fminimum_numl:
+    case Builtin::BI__builtin_fminimum_numf128:
+      return RValue::get(emitBinaryMaybeConstrainedFPBuiltin(
+          *this, E, Intrinsic::minimumnum,
+          Intrinsic::experimental_constrained_minimumnum));
+
     // fmod() is a special-case. It maps to the frem instruction rather than an
     // LLVM intrinsic.
     case Builtin::BIfmod:
diff --git a/clang/lib/Tooling/Inclusions/Stdlib/CSymbolMap.inc b/clang/lib/Tooling/Inclusions/Stdlib/CSymbolMap.inc
index 463ce921f0672..af2dcb632fbb6 100644
--- a/clang/lib/Tooling/Inclusions/Stdlib/CSymbolMap.inc
+++ b/clang/lib/Tooling/Inclusions/Stdlib/CSymbolMap.inc
@@ -475,6 +475,12 @@ SYMBOL(fmaxl, None, <math.h>)
 SYMBOL(fmin, None, <math.h>)
 SYMBOL(fminf, None, <math.h>)
 SYMBOL(fminl, None, <math.h>)
+SYMBOL(fmaximum_num, None, <math.h>)
+SYMBOL(fmaximum_numf, None, <math.h>)
+SYMBOL(fmaximum_numfl, None, <math.h>)
+SYMBOL(fminimum_num, None, <math.h>)
+SYMBOL(fminimum_numf, None, <math.h>)
+SYMBOL(fminimum_numl, None, <math.h>)
 SYMBOL(fmod, None, <math.h>)
 SYMBOL(fmodf, None, <math.h>)
 SYMBOL(fmodl, None, <math.h>)
diff --git a/clang/lib/Tooling/Inclusions/Stdlib/StdSymbolMap.inc b/clang/lib/Tooling/Inclusions/Stdlib/StdSymbolMap.inc
index b46bd2e4d7a4b..442316ce8d4ff 100644
--- a/clang/lib/Tooling/Inclusions/Stdlib/StdSymbolMap.inc
+++ b/clang/lib/Tooling/Inclusions/Stdlib/StdSymbolMap.inc
@@ -1295,6 +1295,24 @@ SYMBOL(fminf, None, <math.h>)
 SYMBOL(fminl, std::, <cmath>)
 SYMBOL(fminl, None, <cmath>)
 SYMBOL(fminl, None, <math.h>)
+SYMBOL(fmaximum_num, std::, <cmath>)
+SYMBOL(fmaximum_num, None, <cmath>)
+SYMBOL(fmaximum_num, None, <math.h>)
+SYMBOL(fmaximum_numf, std::, <cmath>)
+SYMBOL(fmaximum_numf, None, <cmath>)
+SYMBOL(fmaximum_numf, None, <math.h>)
+SYMBOL(fmaximum_numl, std::, <cmath>)
+SYMBOL(fmaximum_numl, None, <cmath>)
+SYMBOL(fmaximum_numl, None, <math.h>)
+SYMBOL(fminimum_num, std::, <cmath>)
+SYMBOL(fminimum_num, None, <cmath>)
+SYMBOL(fminimum_num, None, <math.h>)
+SYMBOL(fminimum_numf, std::, <cmath>)
+SYMBOL(fminimum_numf, None, <cmath>)
+SYMBOL(fminimum_numf, None, <math.h>)
+SYMBOL(fminimum_numl, std::, <cmath>)
+SYMBOL(fminimum_numl, None, <cmath>)
+SYMBOL(fminimum_numl, None, <math.h>)
 SYMBOL(fmod, std::, <cmath>)
 SYMBOL(fmod, None, <cmath>)
 SYMBOL(fmod, None, <math.h>)
diff --git a/clang/test/CodeGen/builtins.c b/clang/test/CodeGen/builtins.c
index b41efb59e61db..b059346bf93d3 100644
--- a/clang/test/CodeGen/builtins.c
+++ b/clang/test/CodeGen/builtins.c
@@ -353,6 +353,24 @@ void test_float_builtin_ops(float F, double D, long double LD) {
   resld = __builtin_fmaxl(LD, LD);
   // CHECK: call x86_fp80 @llvm.maxnum.f80
 
+  resf = __builtin_fminimum_numf(F, F);
+  // CHECK: call float @llvm.minimumnum.f32
+
+  resd = __builtin_fminimum_num(D, D);
+  // CHECK: call double @llvm.minimumnum.f64
+
+  resld = __builtin_fminimum_numl(LD, LD);
+  // CHECK: call x86_fp80 @llvm.minimumnum.f80
+
+  resf = __builtin_fmaximum_numf(F, F);
+  // CHECK: call float @llvm.maximumnum.f32
+
+  resd = __builtin_fmaximum_num(D, D);
+  // CHECK: call double @llvm.maximumnum.f64
+
+  resld = __builtin_fmaximum_numl(LD, LD);
+  // CHECK: call x86_fp80 @llvm.maximumnum.f80
+
   resf = __builtin_fabsf(F);
   // CHECK: call float @llvm.fabs.f32
 
diff --git a/clang/test/CodeGen/math-libcalls.c b/clang/test/CodeGen/math-libcalls.c
index a249182692762..3083287f77b3f 100644
--- a/clang/test/CodeGen/math-libcalls.c
+++ b/clang/test/CodeGen/math-libcalls.c
@@ -372,6 +372,31 @@ void foo(double *d, float f, float *fp, long double *l, int *i, const char *c) {
 // HAS_MAYTRAP: declare float @llvm.experimental.constrained.minnum.f32(
 // HAS_MAYTRAP: declare x86_fp80 @llvm.experimental.constrained.minnum.f80(
 
+  fmaximum_num(f,f);       fmaximum_numf(f,f);      fmaximum_numl(f,f);
+
+// NO__ERRNO: declare double @llvm.maximumnum.f64(double, double) [[READNONE_INTRINSIC]]
+// NO__ERRNO: declare float @llvm.maximumnum.f32(float, float) [[READNONE_INTRINSIC]]
+// NO__ERRNO: declare x86_fp80 @llvm.maximumnum.f80(x86_fp80, x86_fp80) [[READNONE_INTRINSIC]]
+// HAS_ERRNO: declare double @llvm.maximumnum.f64(double, double) [[READNONE_INTRINSIC]]
+// HAS_ERRNO: declare float @llvm.maximumnum.f32(float, float) [[READNONE_INTRINSIC]]
+// HAS_ERRNO: declare x86_fp80 @llvm.maximumnum.f80(x86_fp80, x86_fp80) [[READNONE_INTRINSIC]]
+// HAS_MAYTRAP: declare double @llvm.experimental.constrained.maximumnum.f64(
+// HAS_MAYTRAP: declare float @llvm.experimental.constrained.maximumnum.f32(
+// HAS_MAYTRAP: declare x86_fp80 @llvm.experimental.constrained.maximumnum.f80(
+
+  fminimum_num(f,f);       fminimum_numf(f,f);      fminimum_numl(f,f);
+
+// NO__ERRNO: declare double @llvm.minimumnum.f64(double, double) [[READNONE_INTRINSIC]]
+// NO__ERRNO: declare float @llvm.minimumnum.f32(float, float) [[READNONE_INTRINSIC]]
+// NO__ERRNO: declare x86_fp80 @llvm.minimumnum.f80(x86_fp80, x86_fp80) [[READNONE_INTRINSIC]]
+// HAS_ERRNO: declare double @llvm.minimumnum.f64(double, double) [[READNONE_INTRINSIC]]
+// HAS_ERRNO: declare float @llvm.minimumnum.f32(float, float) [[READNONE_INTRINSIC]]
+// HAS_ERRNO: declare x86_fp80 @llvm.minimumnum.f80(x86_fp80, x86_fp80) [[READNONE_INTRINSIC]]
+// HAS_MAYTRAP: declare double @llvm.experimental.constrained.minimumnum.f64(
+// HAS_MAYTRAP: declare float @llvm.experimental.constrained.minimumnum.f32(
+// HAS_MAYTRAP: declare x86_fp80 @llvm.experimental.constrained.minimumnum.f80(
+
+
   hypot(f,f);      hypotf(f,f);     hypotl(f,f);
 
 // NO__ERRNO: declare double @hypot(double noundef, double noundef) [[READNONE]]

@wzssyqa wzssyqa requested review from philnik777 and hokein June 21, 2024 06:50
Copy link

github-actions bot commented Oct 11, 2024

✅ With the latest revision this PR passed the C/C++ code formatter.

@tbaederr
Copy link
Contributor

Can you add a test that passes a non-float value and checks that it's rejected?

@wzssyqa
Copy link
Contributor Author

wzssyqa commented Oct 11, 2024

I cannot reproduce the fail of Driver/hip-partial-link.hip. Any idea about it?

@tbaederr
Copy link
Contributor

It's not your fault

@wzssyqa
Copy link
Contributor Author

wzssyqa commented Oct 11, 2024

Can you add a test that passes a non-float value and checks that it's rejected?

Do you mean something like this?

$ cat xx.c 
#include <math.h>

float f(char *a, char *b) {
        return fminimum_num(a, b);
}
$ ./bin/clang -std=c23 -O2 -S -emit-llvm xx.c 
xx.c:4:22: error: passing 'char *' to parameter of incompatible type 'double'
    4 |         return fminimum_num(a, b);
      |                             ^
/usr/include/x86_64-linux-gnu/bits/mathcalls.h:409:40: note: passing argument to parameter '__x' here
  409 | __MATHCALLX (fminimum_num,, (_Mdouble_ __x, _Mdouble_ __y), (__const__));
      |                                        ^
1 error generated.

If so, I don't think that we test it here. It is too generic.

@tbaederr
Copy link
Contributor

Can you add a test that passes a non-float value and checks that it's rejected?

Do you mean something like this?

$ cat xx.c 
#include <math.h>

float f(char *a, char *b) {
        return fminimum_num(a, b);
}
$ ./bin/clang -std=c23 -O2 -S -emit-llvm xx.c 
xx.c:4:22: error: passing 'char *' to parameter of incompatible type 'double'
    4 |         return fminimum_num(a, b);
      |                             ^
/usr/include/x86_64-linux-gnu/bits/mathcalls.h:409:40: note: passing argument to parameter '__x' here
  409 | __MATHCALLX (fminimum_num,, (_Mdouble_ __x, _Mdouble_ __y), (__const__));
      |                                        ^
1 error generated.

Yes, but also with an integer please

@wzssyqa
Copy link
Contributor Author

wzssyqa commented Oct 11, 2024

Yes, but also with an integer please

Clang can convert integer to float, so it won't be a failure.

@tbaederr
Copy link
Contributor

Add it as a succcessful test then to check that the EvaluateFloat calls don't fail

@wzssyqa
Copy link
Contributor Author

wzssyqa commented Oct 11, 2024

Add it as a succcessful test then to check that the EvaluateFloat calls don't fail

done.

@wzssyqa
Copy link
Contributor Author

wzssyqa commented Oct 11, 2024

Can you add a test that passes a non-float value and checks that it's rejected?

Do you mean something like this?

$ cat xx.c 
#include <math.h>

float f(char *a, char *b) {
        return fminimum_num(a, b);
}
$ ./bin/clang -std=c23 -O2 -S -emit-llvm xx.c 
xx.c:4:22: error: passing 'char *' to parameter of incompatible type 'double'
    4 |         return fminimum_num(a, b);
      |                             ^
/usr/include/x86_64-linux-gnu/bits/mathcalls.h:409:40: note: passing argument to parameter '__x' here
  409 | __MATHCALLX (fminimum_num,, (_Mdouble_ __x, _Mdouble_ __y), (__const__));
      |                                        ^
1 error generated.

Added. for char *.

@wzssyqa wzssyqa requested a review from arsenm October 12, 2024 01:21
@wzssyqa wzssyqa requested a review from arsenm October 12, 2024 05:37
@arsenm
Copy link
Contributor

arsenm commented Oct 14, 2024

Maybe should add support for elementwise builtins next

@wzssyqa wzssyqa merged commit 5bf81e5 into llvm:main Oct 14, 2024
8 checks passed
@wzssyqa wzssyqa deleted the minimum_num_clang branch October 14, 2024 07:49
@wzssyqa
Copy link
Contributor Author

wzssyqa commented Oct 14, 2024

Maybe should add support for elementwise builtins next

#112164

DanielCChen pushed a commit to DanielCChen/llvm-project that referenced this pull request Oct 16, 2024
We just introduce llvm.minimumnum and llvm.maximumnum intrinsics support
to llvm. Let's support them in Clang.

See: llvm#93033
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:codegen IR generation bugs: mangling, exceptions, etc. clang:frontend Language frontend issues, e.g. anything involving "Sema" clang Clang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants