Skip to content

Clang: Add nsz to llvm.minnum and llvm.maxnum emitted from fmin and fmax #113133

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

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

wzssyqa
Copy link
Contributor

@wzssyqa wzssyqa commented Oct 21, 2024

See: #112852

We will define llvm.minnum and llvm.maxnum with +0.0>-0.0, by default, while libc doesn't require it.

@llvmbot llvmbot added clang Clang issues not falling into any other category clang:codegen IR generation bugs: mangling, exceptions, etc. llvm:ir labels Oct 21, 2024
@llvmbot
Copy link
Member

llvmbot commented Oct 21, 2024

@llvm/pr-subscribers-llvm-ir

@llvm/pr-subscribers-clang-codegen

Author: YunQiang Su (wzssyqa)

Changes

See: #112852

We will define llvm.minnum and llvm.maxnum with +0.0>-0.0, by default, while libc doesn't require it.


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

4 Files Affected:

  • (modified) clang/lib/CodeGen/CGBuiltin.cpp (+20-13)
  • (added) clang/test/CodeGen/fmaxnum_fminnum_use_nsz.c (+33)
  • (modified) llvm/include/llvm/IR/IRBuilder.h (+10-6)
  • (modified) llvm/lib/IR/IRBuilder.cpp (+2-2)
diff --git a/clang/lib/CodeGen/CGBuiltin.cpp b/clang/lib/CodeGen/CGBuiltin.cpp
index 28f28c70b5ae52..f2d6049908720b 100644
--- a/clang/lib/CodeGen/CGBuiltin.cpp
+++ b/clang/lib/CodeGen/CGBuiltin.cpp
@@ -510,19 +510,20 @@ static Value *emitUnaryMaybeConstrainedFPBuiltin(CodeGenFunction &CGF,
 
 // Emit an intrinsic that has 2 operands of the same type as its result.
 // Depending on mode, this may be a constrained floating-point intrinsic.
-static Value *emitBinaryMaybeConstrainedFPBuiltin(CodeGenFunction &CGF,
-                                const CallExpr *E, unsigned IntrinsicID,
-                                unsigned ConstrainedIntrinsicID) {
+static Value *emitBinaryMaybeConstrainedFPBuiltin(
+    CodeGenFunction &CGF, const CallExpr *E, unsigned IntrinsicID,
+    unsigned ConstrainedIntrinsicID, llvm::FastMathFlags *FMF = nullptr) {
   llvm::Value *Src0 = CGF.EmitScalarExpr(E->getArg(0));
   llvm::Value *Src1 = CGF.EmitScalarExpr(E->getArg(1));
 
   CodeGenFunction::CGFPOptionsRAII FPOptsRAII(CGF, E);
   if (CGF.Builder.getIsFPConstrained()) {
     Function *F = CGF.CGM.getIntrinsic(ConstrainedIntrinsicID, Src0->getType());
-    return CGF.Builder.CreateConstrainedFPCall(F, { Src0, Src1 });
+    return CGF.Builder.CreateConstrainedFPCall(F, {Src0, Src1}, "",
+                                               std::nullopt, std::nullopt, FMF);
   } else {
     Function *F = CGF.CGM.getIntrinsic(IntrinsicID, Src0->getType());
-    return CGF.Builder.CreateCall(F, { Src0, Src1 });
+    return CGF.Builder.CreateCall(F, {Src0, Src1}, "", nullptr, FMF);
   }
 }
 
@@ -2846,10 +2847,13 @@ RValue CodeGenFunction::EmitBuiltinExpr(const GlobalDecl GD, unsigned BuiltinID,
     case Builtin::BI__builtin_fmaxf:
     case Builtin::BI__builtin_fmaxf16:
     case Builtin::BI__builtin_fmaxl:
-    case Builtin::BI__builtin_fmaxf128:
-      return RValue::get(emitBinaryMaybeConstrainedFPBuiltin(*this, E,
-                                   Intrinsic::maxnum,
-                                   Intrinsic::experimental_constrained_maxnum));
+    case Builtin::BI__builtin_fmaxf128: {
+      llvm::FastMathFlags FMF;
+      FMF.setNoSignedZeros();
+      return RValue::get(emitBinaryMaybeConstrainedFPBuiltin(
+          *this, E, Intrinsic::maxnum,
+          Intrinsic::experimental_constrained_maxnum, &FMF));
+    }
 
     case Builtin::BIfmin:
     case Builtin::BIfminf:
@@ -2858,10 +2862,13 @@ RValue CodeGenFunction::EmitBuiltinExpr(const GlobalDecl GD, unsigned BuiltinID,
     case Builtin::BI__builtin_fminf:
     case Builtin::BI__builtin_fminf16:
     case Builtin::BI__builtin_fminl:
-    case Builtin::BI__builtin_fminf128:
-      return RValue::get(emitBinaryMaybeConstrainedFPBuiltin(*this, E,
-                                   Intrinsic::minnum,
-                                   Intrinsic::experimental_constrained_minnum));
+    case Builtin::BI__builtin_fminf128: {
+      llvm::FastMathFlags FMF;
+      FMF.setNoSignedZeros();
+      return RValue::get(emitBinaryMaybeConstrainedFPBuiltin(
+          *this, E, Intrinsic::minnum,
+          Intrinsic::experimental_constrained_minnum, &FMF));
+    }
 
     case Builtin::BIfmaximum_num:
     case Builtin::BIfmaximum_numf:
diff --git a/clang/test/CodeGen/fmaxnum_fminnum_use_nsz.c b/clang/test/CodeGen/fmaxnum_fminnum_use_nsz.c
new file mode 100644
index 00000000000000..9798baf0432fea
--- /dev/null
+++ b/clang/test/CodeGen/fmaxnum_fminnum_use_nsz.c
@@ -0,0 +1,33 @@
+// RUN: %clang_cc1 -triple x86_64 %s -emit-llvm -o - 2>&1 | FileCheck %s --check-prefix=CHECK
+
+float fminf (float, float);
+double fmin (double, double);
+long double fminl (long double, long double);
+float fmaxf (float, float);
+double fmax (double, double);
+long double fmaxl (long double, long double);
+
+// CHECK: call nsz float @llvm.minnum.f32
+float fmin1(float a, float b) {
+        return fminf(a, b);
+}
+// CHECK: call nsz double @llvm.minnum.f64
+float fmin2(double a, double b) {
+        return fmin(a, b);
+}
+// CHECK: call nsz x86_fp80 @llvm.minnum.f80
+float fmin3(long double a, long double b) {
+        return fminl(a, b);
+}
+// CHECK: call nsz float @llvm.maxnum.f32
+float fmax1(float a, float b) {
+        return fmaxf(a, b);
+}
+// CHECK: call nsz double @llvm.maxnum.f64
+float fmax2(double a, double b) {
+        return fmax(a, b);
+}
+// CHECK: call nsz x86_fp80 @llvm.maxnum.f80
+float fmax3(long double a, long double b) {
+        return fmaxl(a, b);
+}
diff --git a/llvm/include/llvm/IR/IRBuilder.h b/llvm/include/llvm/IR/IRBuilder.h
index 23fd8350a29b3d..1baca4f003cad6 100644
--- a/llvm/include/llvm/IR/IRBuilder.h
+++ b/llvm/include/llvm/IR/IRBuilder.h
@@ -2438,12 +2438,14 @@ class IRBuilderBase {
 public:
   CallInst *CreateCall(FunctionType *FTy, Value *Callee,
                        ArrayRef<Value *> Args = {}, const Twine &Name = "",
-                       MDNode *FPMathTag = nullptr) {
+                       MDNode *FPMathTag = nullptr,
+                       FastMathFlags *uFMF = nullptr) {
     CallInst *CI = CallInst::Create(FTy, Callee, Args, DefaultOperandBundles);
     if (IsFPConstrained)
       setConstrainedFPCallAttr(CI);
-    if (isa<FPMathOperator>(CI))
-      setFPAttrs(CI, FPMathTag, FMF);
+    if (isa<FPMathOperator>(CI)) {
+      setFPAttrs(CI, FPMathTag, uFMF ? (FMF | *uFMF) : FMF);
+    }
     return Insert(CI, Name);
   }
 
@@ -2459,9 +2461,10 @@ class IRBuilderBase {
   }
 
   CallInst *CreateCall(FunctionCallee Callee, ArrayRef<Value *> Args = {},
-                       const Twine &Name = "", MDNode *FPMathTag = nullptr) {
+                       const Twine &Name = "", MDNode *FPMathTag = nullptr,
+                       FastMathFlags *uFMF = nullptr) {
     return CreateCall(Callee.getFunctionType(), Callee.getCallee(), Args, Name,
-                      FPMathTag);
+                      FPMathTag, uFMF);
   }
 
   CallInst *CreateCall(FunctionCallee Callee, ArrayRef<Value *> Args,
@@ -2474,7 +2477,8 @@ class IRBuilderBase {
   CallInst *CreateConstrainedFPCall(
       Function *Callee, ArrayRef<Value *> Args, const Twine &Name = "",
       std::optional<RoundingMode> Rounding = std::nullopt,
-      std::optional<fp::ExceptionBehavior> Except = std::nullopt);
+      std::optional<fp::ExceptionBehavior> Except = std::nullopt,
+      FastMathFlags *FMF = nullptr);
 
   Value *CreateSelect(Value *C, Value *True, Value *False,
                       const Twine &Name = "", Instruction *MDFrom = nullptr);
diff --git a/llvm/lib/IR/IRBuilder.cpp b/llvm/lib/IR/IRBuilder.cpp
index f340f7aafdc76f..5feaf956b45a97 100644
--- a/llvm/lib/IR/IRBuilder.cpp
+++ b/llvm/lib/IR/IRBuilder.cpp
@@ -1031,7 +1031,7 @@ CallInst *IRBuilderBase::CreateConstrainedFPCmp(
 CallInst *IRBuilderBase::CreateConstrainedFPCall(
     Function *Callee, ArrayRef<Value *> Args, const Twine &Name,
     std::optional<RoundingMode> Rounding,
-    std::optional<fp::ExceptionBehavior> Except) {
+    std::optional<fp::ExceptionBehavior> Except, FastMathFlags *FMF) {
   llvm::SmallVector<Value *, 6> UseArgs;
 
   append_range(UseArgs, Args);
@@ -1040,7 +1040,7 @@ CallInst *IRBuilderBase::CreateConstrainedFPCall(
     UseArgs.push_back(getConstrainedFPRounding(Rounding));
   UseArgs.push_back(getConstrainedFPExcept(Except));
 
-  CallInst *C = CreateCall(Callee, UseArgs, Name);
+  CallInst *C = CreateCall(Callee, UseArgs, Name, nullptr, FMF);
   setConstrainedFPCallAttr(C);
   return C;
 }

@llvmbot
Copy link
Member

llvmbot commented Oct 21, 2024

@llvm/pr-subscribers-clang

Author: YunQiang Su (wzssyqa)

Changes

See: #112852

We will define llvm.minnum and llvm.maxnum with +0.0>-0.0, by default, while libc doesn't require it.


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

4 Files Affected:

  • (modified) clang/lib/CodeGen/CGBuiltin.cpp (+20-13)
  • (added) clang/test/CodeGen/fmaxnum_fminnum_use_nsz.c (+33)
  • (modified) llvm/include/llvm/IR/IRBuilder.h (+10-6)
  • (modified) llvm/lib/IR/IRBuilder.cpp (+2-2)
diff --git a/clang/lib/CodeGen/CGBuiltin.cpp b/clang/lib/CodeGen/CGBuiltin.cpp
index 28f28c70b5ae52..f2d6049908720b 100644
--- a/clang/lib/CodeGen/CGBuiltin.cpp
+++ b/clang/lib/CodeGen/CGBuiltin.cpp
@@ -510,19 +510,20 @@ static Value *emitUnaryMaybeConstrainedFPBuiltin(CodeGenFunction &CGF,
 
 // Emit an intrinsic that has 2 operands of the same type as its result.
 // Depending on mode, this may be a constrained floating-point intrinsic.
-static Value *emitBinaryMaybeConstrainedFPBuiltin(CodeGenFunction &CGF,
-                                const CallExpr *E, unsigned IntrinsicID,
-                                unsigned ConstrainedIntrinsicID) {
+static Value *emitBinaryMaybeConstrainedFPBuiltin(
+    CodeGenFunction &CGF, const CallExpr *E, unsigned IntrinsicID,
+    unsigned ConstrainedIntrinsicID, llvm::FastMathFlags *FMF = nullptr) {
   llvm::Value *Src0 = CGF.EmitScalarExpr(E->getArg(0));
   llvm::Value *Src1 = CGF.EmitScalarExpr(E->getArg(1));
 
   CodeGenFunction::CGFPOptionsRAII FPOptsRAII(CGF, E);
   if (CGF.Builder.getIsFPConstrained()) {
     Function *F = CGF.CGM.getIntrinsic(ConstrainedIntrinsicID, Src0->getType());
-    return CGF.Builder.CreateConstrainedFPCall(F, { Src0, Src1 });
+    return CGF.Builder.CreateConstrainedFPCall(F, {Src0, Src1}, "",
+                                               std::nullopt, std::nullopt, FMF);
   } else {
     Function *F = CGF.CGM.getIntrinsic(IntrinsicID, Src0->getType());
-    return CGF.Builder.CreateCall(F, { Src0, Src1 });
+    return CGF.Builder.CreateCall(F, {Src0, Src1}, "", nullptr, FMF);
   }
 }
 
@@ -2846,10 +2847,13 @@ RValue CodeGenFunction::EmitBuiltinExpr(const GlobalDecl GD, unsigned BuiltinID,
     case Builtin::BI__builtin_fmaxf:
     case Builtin::BI__builtin_fmaxf16:
     case Builtin::BI__builtin_fmaxl:
-    case Builtin::BI__builtin_fmaxf128:
-      return RValue::get(emitBinaryMaybeConstrainedFPBuiltin(*this, E,
-                                   Intrinsic::maxnum,
-                                   Intrinsic::experimental_constrained_maxnum));
+    case Builtin::BI__builtin_fmaxf128: {
+      llvm::FastMathFlags FMF;
+      FMF.setNoSignedZeros();
+      return RValue::get(emitBinaryMaybeConstrainedFPBuiltin(
+          *this, E, Intrinsic::maxnum,
+          Intrinsic::experimental_constrained_maxnum, &FMF));
+    }
 
     case Builtin::BIfmin:
     case Builtin::BIfminf:
@@ -2858,10 +2862,13 @@ RValue CodeGenFunction::EmitBuiltinExpr(const GlobalDecl GD, unsigned BuiltinID,
     case Builtin::BI__builtin_fminf:
     case Builtin::BI__builtin_fminf16:
     case Builtin::BI__builtin_fminl:
-    case Builtin::BI__builtin_fminf128:
-      return RValue::get(emitBinaryMaybeConstrainedFPBuiltin(*this, E,
-                                   Intrinsic::minnum,
-                                   Intrinsic::experimental_constrained_minnum));
+    case Builtin::BI__builtin_fminf128: {
+      llvm::FastMathFlags FMF;
+      FMF.setNoSignedZeros();
+      return RValue::get(emitBinaryMaybeConstrainedFPBuiltin(
+          *this, E, Intrinsic::minnum,
+          Intrinsic::experimental_constrained_minnum, &FMF));
+    }
 
     case Builtin::BIfmaximum_num:
     case Builtin::BIfmaximum_numf:
diff --git a/clang/test/CodeGen/fmaxnum_fminnum_use_nsz.c b/clang/test/CodeGen/fmaxnum_fminnum_use_nsz.c
new file mode 100644
index 00000000000000..9798baf0432fea
--- /dev/null
+++ b/clang/test/CodeGen/fmaxnum_fminnum_use_nsz.c
@@ -0,0 +1,33 @@
+// RUN: %clang_cc1 -triple x86_64 %s -emit-llvm -o - 2>&1 | FileCheck %s --check-prefix=CHECK
+
+float fminf (float, float);
+double fmin (double, double);
+long double fminl (long double, long double);
+float fmaxf (float, float);
+double fmax (double, double);
+long double fmaxl (long double, long double);
+
+// CHECK: call nsz float @llvm.minnum.f32
+float fmin1(float a, float b) {
+        return fminf(a, b);
+}
+// CHECK: call nsz double @llvm.minnum.f64
+float fmin2(double a, double b) {
+        return fmin(a, b);
+}
+// CHECK: call nsz x86_fp80 @llvm.minnum.f80
+float fmin3(long double a, long double b) {
+        return fminl(a, b);
+}
+// CHECK: call nsz float @llvm.maxnum.f32
+float fmax1(float a, float b) {
+        return fmaxf(a, b);
+}
+// CHECK: call nsz double @llvm.maxnum.f64
+float fmax2(double a, double b) {
+        return fmax(a, b);
+}
+// CHECK: call nsz x86_fp80 @llvm.maxnum.f80
+float fmax3(long double a, long double b) {
+        return fmaxl(a, b);
+}
diff --git a/llvm/include/llvm/IR/IRBuilder.h b/llvm/include/llvm/IR/IRBuilder.h
index 23fd8350a29b3d..1baca4f003cad6 100644
--- a/llvm/include/llvm/IR/IRBuilder.h
+++ b/llvm/include/llvm/IR/IRBuilder.h
@@ -2438,12 +2438,14 @@ class IRBuilderBase {
 public:
   CallInst *CreateCall(FunctionType *FTy, Value *Callee,
                        ArrayRef<Value *> Args = {}, const Twine &Name = "",
-                       MDNode *FPMathTag = nullptr) {
+                       MDNode *FPMathTag = nullptr,
+                       FastMathFlags *uFMF = nullptr) {
     CallInst *CI = CallInst::Create(FTy, Callee, Args, DefaultOperandBundles);
     if (IsFPConstrained)
       setConstrainedFPCallAttr(CI);
-    if (isa<FPMathOperator>(CI))
-      setFPAttrs(CI, FPMathTag, FMF);
+    if (isa<FPMathOperator>(CI)) {
+      setFPAttrs(CI, FPMathTag, uFMF ? (FMF | *uFMF) : FMF);
+    }
     return Insert(CI, Name);
   }
 
@@ -2459,9 +2461,10 @@ class IRBuilderBase {
   }
 
   CallInst *CreateCall(FunctionCallee Callee, ArrayRef<Value *> Args = {},
-                       const Twine &Name = "", MDNode *FPMathTag = nullptr) {
+                       const Twine &Name = "", MDNode *FPMathTag = nullptr,
+                       FastMathFlags *uFMF = nullptr) {
     return CreateCall(Callee.getFunctionType(), Callee.getCallee(), Args, Name,
-                      FPMathTag);
+                      FPMathTag, uFMF);
   }
 
   CallInst *CreateCall(FunctionCallee Callee, ArrayRef<Value *> Args,
@@ -2474,7 +2477,8 @@ class IRBuilderBase {
   CallInst *CreateConstrainedFPCall(
       Function *Callee, ArrayRef<Value *> Args, const Twine &Name = "",
       std::optional<RoundingMode> Rounding = std::nullopt,
-      std::optional<fp::ExceptionBehavior> Except = std::nullopt);
+      std::optional<fp::ExceptionBehavior> Except = std::nullopt,
+      FastMathFlags *FMF = nullptr);
 
   Value *CreateSelect(Value *C, Value *True, Value *False,
                       const Twine &Name = "", Instruction *MDFrom = nullptr);
diff --git a/llvm/lib/IR/IRBuilder.cpp b/llvm/lib/IR/IRBuilder.cpp
index f340f7aafdc76f..5feaf956b45a97 100644
--- a/llvm/lib/IR/IRBuilder.cpp
+++ b/llvm/lib/IR/IRBuilder.cpp
@@ -1031,7 +1031,7 @@ CallInst *IRBuilderBase::CreateConstrainedFPCmp(
 CallInst *IRBuilderBase::CreateConstrainedFPCall(
     Function *Callee, ArrayRef<Value *> Args, const Twine &Name,
     std::optional<RoundingMode> Rounding,
-    std::optional<fp::ExceptionBehavior> Except) {
+    std::optional<fp::ExceptionBehavior> Except, FastMathFlags *FMF) {
   llvm::SmallVector<Value *, 6> UseArgs;
 
   append_range(UseArgs, Args);
@@ -1040,7 +1040,7 @@ CallInst *IRBuilderBase::CreateConstrainedFPCall(
     UseArgs.push_back(getConstrainedFPRounding(Rounding));
   UseArgs.push_back(getConstrainedFPExcept(Except));
 
-  CallInst *C = CreateCall(Callee, UseArgs, Name);
+  CallInst *C = CreateCall(Callee, UseArgs, Name, nullptr, FMF);
   setConstrainedFPCallAttr(C);
   return C;
 }

@wzssyqa wzssyqa requested a review from arsenm October 21, 2024 07:25
@arsenm arsenm added the floating-point Floating-point math label Oct 21, 2024
Copy link
Contributor

@arsenm arsenm left a comment

Choose a reason for hiding this comment

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

We should also add new builtins (with the minnum/maxnum names) to get the stronger 2019 behavior

@wzssyqa wzssyqa force-pushed the clang-fmin-with-nsz branch from e3bff67 to 8655cb0 Compare October 23, 2024 03:11
@wzssyqa wzssyqa requested a review from arsenm October 23, 2024 04:48
@wzssyqa
Copy link
Contributor Author

wzssyqa commented Nov 25, 2024

@arsenm ping

Copy link
Contributor

@jcranmer-intel jcranmer-intel left a comment

Choose a reason for hiding this comment

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

You'll want to merge the fast-math flags, so that compiling with -ffinite-math-only gets you nnan ninf nsz on the maxnum/minnum, rather than just nsz.

@wzssyqa wzssyqa force-pushed the clang-fmin-with-nsz branch from 105c9a7 to a3f3567 Compare February 27, 2025 10:06
@wzssyqa wzssyqa marked this pull request as draft February 27, 2025 10:13
@wzssyqa
Copy link
Contributor Author

wzssyqa commented Feb 27, 2025

nsz is missing for __builtin_elementwise_min.
I am working on it.

@arsenm
Copy link
Contributor

arsenm commented Feb 27, 2025

We should also have new builtins for the raw minnum / maxnum intrinsics (plus elementwise) to not get the nsz

@arsenm
Copy link
Contributor

arsenm commented Feb 27, 2025

nsz is missing for __builtin_elementwise_min. I am working on it.

I'm not sure that's a good idea. For fmin/fmax it seems OK, but this doesn't take the libm name

@wzssyqa wzssyqa force-pushed the clang-fmin-with-nsz branch from 8b63af7 to ce675ee Compare March 4, 2025 06:15
@wzssyqa wzssyqa marked this pull request as ready for review March 4, 2025 06:16
@wzssyqa wzssyqa requested review from arsenm and jcranmer-intel March 4, 2025 06:16
@wzssyqa
Copy link
Contributor Author

wzssyqa commented Mar 4, 2025

You'll want to merge the fast-math flags, so that compiling with -ffinite-math-only gets you nnan ninf nsz on the maxnum/minnum, rather than just nsz.

Done, for cc1, we use -menable-no-nans and -menable-no-infs.

@arsenm arsenm changed the title Clang: emit llvm.minnum and llvm.maxnum with nsz always Clang: Add nsz to llvm.minnum and llvm.maxnum emitted from fmin and fmax Mar 4, 2025
@wzssyqa wzssyqa requested a review from arsenm March 4, 2025 07:49
@wzssyqa wzssyqa requested a review from arsenm March 19, 2025 01:51
@wzssyqa wzssyqa force-pushed the clang-fmin-with-nsz branch from 77267f8 to 5ea0ac7 Compare March 19, 2025 01:54
@wzssyqa wzssyqa force-pushed the clang-fmin-with-nsz branch 2 times, most recently from 4cddbef to 074a74e Compare April 16, 2025 09:34
@wzssyqa wzssyqa force-pushed the clang-fmin-with-nsz branch from 074a74e to a13c65b Compare April 25, 2025 17:22
@nikic
Copy link
Contributor

nikic commented Apr 25, 2025

I'm not sure this is correct. At least the way alive models the flag, I believe this would also allow returning -0.0 even if both operands are 0.0, which is not a legal outcome.

@arsenm
Copy link
Contributor

arsenm commented Apr 25, 2025

I'm not sure this is correct. At least the way alive models the flag, I believe this would also allow returning -0.0 even if both operands are 0.0, which is not a legal outcome.

That sounds like a bad modeling. It shouldn't permit synthesizing a -0 out of nowhere

@nikic
Copy link
Contributor

nikic commented Apr 25, 2025

I'm not sure this is correct. At least the way alive models the flag, I believe this would also allow returning -0.0 even if both operands are 0.0, which is not a legal outcome.

That sounds like a bad modeling. It shouldn't permit synthesizing a -0 out of nowhere

Hm, how would you suggest to model it? Considering an example like https://discourse.llvm.org/t/rfc-clarify-the-behavior-of-fp-operations-on-bit-strings-with-nsz-flag/85981/4?u=nikic, looking at it from the perspective of the select only, requires it to convert a 0.0 into -0.0 "out of nowhere".

I guess we could specify the semantics of nsz per-operation and use something different for minnum...

@wzssyqa
Copy link
Contributor Author

wzssyqa commented Apr 26, 2025

I'm not sure this is correct. At least the way alive models the flag, I believe this would also allow returning -0.0 even if both operands are 0.0, which is not a legal outcome.

That sounds like a bad modeling. It shouldn't permit synthesizing a -0 out of nowhere

Hm, how would you suggest to model it? Considering an example like https://discourse.llvm.org/t/rfc-clarify-the-behavior-of-fp-operations-on-bit-strings-with-nsz-flag/85981/4?u=nikic, looking at it from the perspective of the select only, requires it to convert a 0.0 into -0.0 "out of nowhere".

I guess we could specify the semantics of nsz per-operation and use something different for minnum...

In fact we already do so.

If the intrinsic is marked with the nsz attribute, then the effect is as in the definition in C and IEEE-754-2008: the result of minnum(-0.0, +0.0) may be either -0.0 or +0.0.

wzssyqa added 2 commits April 26, 2025 20:39
See: llvm#112852

We will define llvm.minnum and llvm.maxnum with +0.0>-0.0, by default,
while libc doesn't require it.

fix testcases

-ffp-exception-behavior=strict

add missing builtin test

test auto vectorize

fix test cases

update testcase

disable-llvm-passes

fix elementswise

fix some tests
@wzssyqa wzssyqa force-pushed the clang-fmin-with-nsz branch 2 times, most recently from efb442d to 9753074 Compare April 26, 2025 12:43
@wzssyqa wzssyqa force-pushed the clang-fmin-with-nsz branch from 9753074 to bb4a5fd Compare April 26, 2025 13:10
@efriedma-quic
Copy link
Collaborator

Please propose a fix for the definition of nsz itself itself in LangRef; burying an exception to the general nsz rules in the middle of the definition of min/max is, at best, confusing.

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 Clang issues not falling into any other category floating-point Floating-point math llvm:ir
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants