-
Notifications
You must be signed in to change notification settings - Fork 13.6k
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
base: main
Are you sure you want to change the base?
Conversation
@llvm/pr-subscribers-llvm-ir @llvm/pr-subscribers-clang-codegen Author: YunQiang Su (wzssyqa) ChangesSee: #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:
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;
}
|
@llvm/pr-subscribers-clang Author: YunQiang Su (wzssyqa) ChangesSee: #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:
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;
}
|
There was a problem hiding this 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
e3bff67
to
8655cb0
Compare
@arsenm ping |
There was a problem hiding this 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.
105c9a7
to
a3f3567
Compare
nsz is missing for |
We should also have new builtins for the raw minnum / maxnum intrinsics (plus elementwise) to not get the nsz |
I'm not sure that's a good idea. For fmin/fmax it seems OK, but this doesn't take the libm name |
8b63af7
to
ce675ee
Compare
Done, for cc1, we use |
77267f8
to
5ea0ac7
Compare
4cddbef
to
074a74e
Compare
074a74e
to
a13c65b
Compare
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.
|
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
efb442d
to
9753074
Compare
9753074
to
bb4a5fd
Compare
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. |
See: #112852
We will define llvm.minnum and llvm.maxnum with +0.0>-0.0, by default, while libc doesn't require it.