Skip to content

Conversation

@wzssyqa
Copy link
Contributor

@wzssyqa wzssyqa commented Feb 28, 2025

In #112852, we claimed that llvm.minnum and llvm.maxnum should treat +0.0>-0.0, while libc doesn't require fmin(3)/fmax(3) for it.

Let's add FMFSource parameter to CreateMaxNum and CreateMinNum, so that they can be used by CodeGenFunction::EmitBuiltinExpr of Clang.

@wzssyqa wzssyqa requested a review from arsenm February 28, 2025 01:51
@llvmbot
Copy link
Member

llvmbot commented Feb 28, 2025

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

@llvm/pr-subscribers-llvm-ir

Author: YunQiang Su (wzssyqa)

Changes

In #112852, we claimed that llvm.minnum and llvm.maxnum should treat +0.0>-0.0, while libc doesn't require fmin(3)/fmax(3) for it.

Let's add NoSignedZeros parameter to CreateMaxNum and CreateMinNum, so that they can used by CodeGenFunction::EmitBuiltinExpr of Clang.


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

1 Files Affected:

  • (modified) llvm/include/llvm/IR/IRBuilder.h (+14-6)
diff --git a/llvm/include/llvm/IR/IRBuilder.h b/llvm/include/llvm/IR/IRBuilder.h
index 933dbb306d1fc..fe8c269101847 100644
--- a/llvm/include/llvm/IR/IRBuilder.h
+++ b/llvm/include/llvm/IR/IRBuilder.h
@@ -1005,23 +1005,31 @@ class IRBuilderBase {
                             const Twine &Name = "");
 
   /// Create call to the minnum intrinsic.
-  Value *CreateMinNum(Value *LHS, Value *RHS, const Twine &Name = "") {
+  Value *CreateMinNum(Value *LHS, Value *RHS, const Twine &Name = "",
+                      bool NoSignedZeros = false) {
+    llvm::FastMathFlags FMF;
+    FMF.setNoSignedZeros(NoSignedZeros);
+    FMFSource FMFSrc(FMF);
     if (IsFPConstrained) {
       return CreateConstrainedFPUnroundedBinOp(
-          Intrinsic::experimental_constrained_minnum, LHS, RHS, nullptr, Name);
+          Intrinsic::experimental_constrained_minnum, LHS, RHS, FMFSrc, Name);
     }
 
-    return CreateBinaryIntrinsic(Intrinsic::minnum, LHS, RHS, nullptr, Name);
+    return CreateBinaryIntrinsic(Intrinsic::minnum, LHS, RHS, FMFSrc, Name);
   }
 
   /// Create call to the maxnum intrinsic.
-  Value *CreateMaxNum(Value *LHS, Value *RHS, const Twine &Name = "") {
+  Value *CreateMaxNum(Value *LHS, Value *RHS, const Twine &Name = "",
+                      bool NoSignedZeros = false) {
+    llvm::FastMathFlags FMF;
+    FMF.setNoSignedZeros(NoSignedZeros);
+    FMFSource FMFSrc(FMF);
     if (IsFPConstrained) {
       return CreateConstrainedFPUnroundedBinOp(
-          Intrinsic::experimental_constrained_maxnum, LHS, RHS, nullptr, Name);
+          Intrinsic::experimental_constrained_maxnum, LHS, RHS, FMFSrc, Name);
     }
 
-    return CreateBinaryIntrinsic(Intrinsic::maxnum, LHS, RHS, nullptr, Name);
+    return CreateBinaryIntrinsic(Intrinsic::maxnum, LHS, RHS, FMFSrc, Name);
   }
 
   /// Create call to the minimum intrinsic.

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.

Should add a general FastMathFlags argument

@wzssyqa wzssyqa changed the title IRBuilder: Add NoSignedZeros parameter to CreateMaxNum/CreateMinNum IRBuilder: Add FastMathFlags parameter to CreateMaxNum/CreateMinNum Feb 28, 2025
@wzssyqa wzssyqa requested a review from arsenm February 28, 2025 02:23
@arsenm arsenm requested a review from dtcxzyw February 28, 2025 05:33
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.

Should follow the precedent set by #121657 and use FMFSource

@wzssyqa wzssyqa changed the title IRBuilder: Add FastMathFlags parameter to CreateMaxNum/CreateMinNum IRBuilder: Add FMFSourceparameter to CreateMaxNum/CreateMinNum Feb 28, 2025
@wzssyqa wzssyqa changed the title IRBuilder: Add FMFSourceparameter to CreateMaxNum/CreateMinNum IRBuilder: Add FMFSource parameter to CreateMaxNum/CreateMinNum Feb 28, 2025
In llvm#112852, we claimed that
llvm.minnum and llvm.maxnum should treat +0.0>-0.0, while libc doesn't
require fmin(3)/fmax(3) for it.

Let's add FMFSource parameter to CreateMaxNum and CreateMinNum,
so that it can be used by CodeGenFunction::EmitBuiltinExpr of Clang.
@github-actions
Copy link

github-actions bot commented Feb 28, 2025

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

@llvmbot llvmbot added clang Clang issues not falling into any other category clang:codegen IR generation bugs: mangling, exceptions, etc. labels Mar 3, 2025
@wzssyqa wzssyqa requested a review from arsenm March 3, 2025 07:12
@wzssyqa wzssyqa merged commit 4bd3427 into llvm:main Mar 4, 2025
13 checks passed
@wzssyqa wzssyqa deleted the CreateMinNum branch March 4, 2025 00:49
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 llvm:ir

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants