[clang] [llvm] Clang: Add nsz to llvm.minnum and llvm.maxnum emitted from fmin and fmax (PR #113133)
https://github.com/nikic approved this pull request. LGTM https://github.com/llvm/llvm-project/pull/113133 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] Clang: Add nsz to llvm.minnum and llvm.maxnum emitted from fmin and fmax (PR #113133)
nikic wrote: > > What do you mean by "newer operations"? Do you mean the > > fminimum/fminimum_num functions? The PR doesn't touch those. Or are you > > talking about `__builtin_elementwise_min` (which the PR currently does set > > nsz on)? > > Yes, __builtin_elementwise_min and __builtin_elementwise_minnum Okay, I'm fine with that. Though IMHO we should also remove FP support from __builtin_elementwise_min entirely. https://github.com/llvm/llvm-project/pull/113133 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] Clang: Add nsz to llvm.minnum and llvm.maxnum emitted from fmin and fmax (PR #113133)
arsenm wrote: > What do you mean by "newer operations"? Do you mean the fminimum/fminimum_num > functions? The PR doesn't touch those. Or are you talking about > `__builtin_elementwise_min` (which the PR currently does set nsz on)? Yes, __builtin_elementwise_min and __builtin_elementwise_minnum https://github.com/llvm/llvm-project/pull/113133 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] Clang: Add nsz to llvm.minnum and llvm.maxnum emitted from fmin and fmax (PR #113133)
@@ -4002,8 +4012,11 @@ RValue CodeGenFunction::EmitBuiltinExpr(const GlobalDecl
GD, unsigned BuiltinID,
Result = Builder.CreateBinaryIntrinsic(
Ty->isSignedIntegerType() ? Intrinsic::smin : Intrinsic::umin, Op0,
Op1, nullptr, "elt.min");
-} else
- Result = Builder.CreateMinNum(Op0, Op1, /*FMFSource=*/nullptr,
"elt.min");
+} else {
+ FastMathFlags FMF;
+ FMF.setNoSignedZeros(true);
wzssyqa wrote:
> I agree this should not continue to use minnum/maxnum, though that change
> should not be part of this PR.
>
So can we merge this PR?
> minnum/maxnum behavior has never been consistent across targets (or across
> scenarios).
In fact, all of the architectures that claims implement IEEE754-2008, has the
same behavior:
AArch64, MIPSr6, LoongArch, PowerPC/VSX
That's why I'd plan to define `minnum/maxnum` as the same as these
architectures.
> But we could also add
> __builtin_elementwise_maximumnum/__builtin_elementwise_minimnumnum
I will do it.
https://github.com/llvm/llvm-project/pull/113133
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] Clang: Add nsz to llvm.minnum and llvm.maxnum emitted from fmin and fmax (PR #113133)
@@ -4002,8 +4012,11 @@ RValue CodeGenFunction::EmitBuiltinExpr(const GlobalDecl
GD, unsigned BuiltinID,
Result = Builder.CreateBinaryIntrinsic(
Ty->isSignedIntegerType() ? Intrinsic::smin : Intrinsic::umin, Op0,
Op1, nullptr, "elt.min");
-} else
- Result = Builder.CreateMinNum(Op0, Op1, /*FMFSource=*/nullptr,
"elt.min");
+} else {
+ FastMathFlags FMF;
+ FMF.setNoSignedZeros(true);
wzssyqa wrote:
In fact I am planning to add `__builtin_elementwise_maximumnum` after this PR
is merged.
https://github.com/llvm/llvm-project/pull/113133
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] Clang: Add nsz to llvm.minnum and llvm.maxnum emitted from fmin and fmax (PR #113133)
https://github.com/wzssyqa edited https://github.com/llvm/llvm-project/pull/113133 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] Clang: Add nsz to llvm.minnum and llvm.maxnum emitted from fmin and fmax (PR #113133)
@@ -4002,8 +4012,11 @@ RValue CodeGenFunction::EmitBuiltinExpr(const GlobalDecl
GD, unsigned BuiltinID,
Result = Builder.CreateBinaryIntrinsic(
Ty->isSignedIntegerType() ? Intrinsic::smin : Intrinsic::umin, Op0,
Op1, nullptr, "elt.min");
-} else
- Result = Builder.CreateMinNum(Op0, Op1, /*FMFSource=*/nullptr,
"elt.min");
+} else {
+ FastMathFlags FMF;
+ FMF.setNoSignedZeros(true);
wzssyqa wrote:
Why not introduce `__builtin_elementwise_maximumnum`?
In practice, I'd prefer keep the behavior of current function and introduce new
functions to solve the `non-associative` problem.
Libc does do like this.
https://github.com/llvm/llvm-project/pull/113133
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] Clang: Add nsz to llvm.minnum and llvm.maxnum emitted from fmin and fmax (PR #113133)
@@ -4002,8 +4012,11 @@ RValue CodeGenFunction::EmitBuiltinExpr(const GlobalDecl
GD, unsigned BuiltinID,
Result = Builder.CreateBinaryIntrinsic(
Ty->isSignedIntegerType() ? Intrinsic::smin : Intrinsic::umin, Op0,
Op1, nullptr, "elt.min");
-} else
- Result = Builder.CreateMinNum(Op0, Op1, /*FMFSource=*/nullptr,
"elt.min");
+} else {
+ FastMathFlags FMF;
+ FMF.setNoSignedZeros(true);
nikic wrote:
I think `__builtin_elementwise_min` must be changed to use `minimumnum`
semantics to be coherent now. The new `minnum` semantics are non-associative,
so an elementwise minnum is no longer a well-defined operation. It would have
to specify an explicit reduction order, which requires a different intrinsic
design (see the signature differences between reduce.fadd and reduce.fmin).
https://github.com/llvm/llvm-project/pull/113133
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] Clang: Add nsz to llvm.minnum and llvm.maxnum emitted from fmin and fmax (PR #113133)
https://github.com/wzssyqa deleted https://github.com/llvm/llvm-project/pull/113133 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] Clang: Add nsz to llvm.minnum and llvm.maxnum emitted from fmin and fmax (PR #113133)
@@ -4002,8 +4012,11 @@ RValue CodeGenFunction::EmitBuiltinExpr(const GlobalDecl
GD, unsigned BuiltinID,
Result = Builder.CreateBinaryIntrinsic(
Ty->isSignedIntegerType() ? Intrinsic::smin : Intrinsic::umin, Op0,
Op1, nullptr, "elt.min");
-} else
- Result = Builder.CreateMinNum(Op0, Op1, /*FMFSource=*/nullptr,
"elt.min");
+} else {
+ FastMathFlags FMF;
+ FMF.setNoSignedZeros(true);
wzssyqa wrote:
> But they are in fact, different operations. And you have many choices for
> which FP min/max.
>
> Given this name doesn't have the historic fmin/fmax in it, I don't think this
> should take the fuzzy signed zero handling
You are right, since we have
`__builtin_elementwise_max/__builtin_elementwise_min`, we should not add `nsz`
to
`maxnum/minnum`.
https://github.com/llvm/llvm-project/pull/113133
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] Clang: Add nsz to llvm.minnum and llvm.maxnum emitted from fmin and fmax (PR #113133)
@@ -4002,8 +4012,11 @@ RValue CodeGenFunction::EmitBuiltinExpr(const GlobalDecl
GD, unsigned BuiltinID,
Result = Builder.CreateBinaryIntrinsic(
Ty->isSignedIntegerType() ? Intrinsic::smin : Intrinsic::umin, Op0,
Op1, nullptr, "elt.min");
-} else
- Result = Builder.CreateMinNum(Op0, Op1, /*FMFSource=*/nullptr,
"elt.min");
+} else {
+ FastMathFlags FMF;
+ FMF.setNoSignedZeros(true);
wzssyqa wrote:
maybe we should add
```
__builtin_elementwise_maximumnum
__builtin_elementwise_minimumnum
```
in future.
https://github.com/llvm/llvm-project/pull/113133
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] Clang: Add nsz to llvm.minnum and llvm.maxnum emitted from fmin and fmax (PR #113133)
@@ -4002,8 +4012,11 @@ RValue CodeGenFunction::EmitBuiltinExpr(const GlobalDecl
GD, unsigned BuiltinID,
Result = Builder.CreateBinaryIntrinsic(
Ty->isSignedIntegerType() ? Intrinsic::smin : Intrinsic::umin, Op0,
Op1, nullptr, "elt.min");
-} else
- Result = Builder.CreateMinNum(Op0, Op1, /*FMFSource=*/nullptr,
"elt.min");
+} else {
+ FastMathFlags FMF;
+ FMF.setNoSignedZeros(true);
wzssyqa wrote:
You can notice that we have already:
```
__builtin_elementwise_maximum
__builtin_elementwise_minimum
```
before https://github.com/llvm/llvm-project/pull/129207
And with https://github.com/llvm/llvm-project/pull/129207
we introduce
```
__builtin_elementwise_maxnum
__builtin_elementwise_minnum
```
So in the best, we should keep the name scheme: not prefix `f`.
https://github.com/llvm/llvm-project/pull/113133
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] Clang: Add nsz to llvm.minnum and llvm.maxnum emitted from fmin and fmax (PR #113133)
@@ -4002,8 +4012,11 @@ RValue CodeGenFunction::EmitBuiltinExpr(const GlobalDecl
GD, unsigned BuiltinID,
Result = Builder.CreateBinaryIntrinsic(
Ty->isSignedIntegerType() ? Intrinsic::smin : Intrinsic::umin, Op0,
Op1, nullptr, "elt.min");
-} else
- Result = Builder.CreateMinNum(Op0, Op1, /*FMFSource=*/nullptr,
"elt.min");
+} else {
+ FastMathFlags FMF;
+ FMF.setNoSignedZeros(true);
fhahn wrote:
Yes this should be discussed separately. Feedback from library authors has been
that different builtins for floats/ints are a bit of a pain
https://github.com/llvm/llvm-project/pull/113133
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] Clang: Add nsz to llvm.minnum and llvm.maxnum emitted from fmin and fmax (PR #113133)
@@ -4002,8 +4012,11 @@ RValue CodeGenFunction::EmitBuiltinExpr(const GlobalDecl
GD, unsigned BuiltinID,
Result = Builder.CreateBinaryIntrinsic(
Ty->isSignedIntegerType() ? Intrinsic::smin : Intrinsic::umin, Op0,
Op1, nullptr, "elt.min");
-} else
- Result = Builder.CreateMinNum(Op0, Op1, /*FMFSource=*/nullptr,
"elt.min");
+} else {
+ FastMathFlags FMF;
+ FMF.setNoSignedZeros(true);
wzssyqa wrote:
If we are planning to drop float support of __builtin_elementwise, it should be
in another patchset.
https://github.com/llvm/llvm-project/pull/113133
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] Clang: Add nsz to llvm.minnum and llvm.maxnum emitted from fmin and fmax (PR #113133)
https://github.com/wzssyqa edited https://github.com/llvm/llvm-project/pull/113133 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] Clang: Add nsz to llvm.minnum and llvm.maxnum emitted from fmin and fmax (PR #113133)
@@ -4002,8 +4012,11 @@ RValue CodeGenFunction::EmitBuiltinExpr(const GlobalDecl
GD, unsigned BuiltinID,
Result = Builder.CreateBinaryIntrinsic(
Ty->isSignedIntegerType() ? Intrinsic::smin : Intrinsic::umin, Op0,
Op1, nullptr, "elt.min");
-} else
- Result = Builder.CreateMinNum(Op0, Op1, /*FMFSource=*/nullptr,
"elt.min");
+} else {
+ FastMathFlags FMF;
+ FMF.setNoSignedZeros(true);
wzssyqa wrote:
See: https://github.com/llvm/llvm-project/pull/129207
In that PR, we planned to use the same naming scheme
__builtin_elementwise_max -> fmax
https://github.com/llvm/llvm-project/pull/113133
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] Clang: Add nsz to llvm.minnum and llvm.maxnum emitted from fmin and fmax (PR #113133)
@@ -4002,8 +4012,11 @@ RValue CodeGenFunction::EmitBuiltinExpr(const GlobalDecl
GD, unsigned BuiltinID,
Result = Builder.CreateBinaryIntrinsic(
Ty->isSignedIntegerType() ? Intrinsic::smin : Intrinsic::umin, Op0,
Op1, nullptr, "elt.min");
-} else
- Result = Builder.CreateMinNum(Op0, Op1, /*FMFSource=*/nullptr,
"elt.min");
+} else {
+ FastMathFlags FMF;
+ FMF.setNoSignedZeros(true);
arsenm wrote:
I don't think this should apply in the elementwise case.
I also think it was a mistake to allow floating point in elementwise min/max
https://github.com/llvm/llvm-project/pull/113133
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] Clang: Add nsz to llvm.minnum and llvm.maxnum emitted from fmin and fmax (PR #113133)
github-actions[bot] wrote:
:warning: C/C++ code formatter, clang-format found issues in your code.
:warning:
You can test this locally with the following command:
``bash
git-clang-format --diff origin/main HEAD --extensions h,c,cpp,cl --
clang/lib/CodeGen/CGBuiltin.cpp clang/test/CodeGen/RISCV/math-builtins.c
clang/test/CodeGen/builtins-elementwise-math.c clang/test/CodeGen/builtins.c
clang/test/CodeGen/constrained-math-builtins.c
clang/test/CodeGen/math-builtins-long.c
clang/test/CodeGen/strictfp-elementwise-builtins.cpp
clang/test/CodeGenOpenCL/builtins-f16.cl
clang/test/Headers/amdgcn_openmp_device_math_constexpr.cpp
llvm/include/llvm/IR/IRBuilder.h llvm/lib/IR/IRBuilder.cpp
--diff_from_common_commit
``
:warning:
The reproduction instructions above might return results for more than one PR
in a stack if you are using a stacked PR workflow. You can limit the results by
changing `origin/main` to the base branch/commit you want to compare against.
:warning:
View the diff from clang-format here.
``diff
diff --git a/clang/lib/CodeGen/CGBuiltin.cpp b/clang/lib/CodeGen/CGBuiltin.cpp
index e55699d5e..c9955a96b 100644
--- a/clang/lib/CodeGen/CGBuiltin.cpp
+++ b/clang/lib/CodeGen/CGBuiltin.cpp
@@ -611,8 +611,8 @@ static Value *emitBinaryMaybeConstrainedFPBuiltin(
CodeGenFunction::CGFPOptionsRAII FPOptsRAII(CGF, E);
if (CGF.Builder.getIsFPConstrained()) {
Function *F = CGF.CGM.getIntrinsic(ConstrainedIntrinsicID,
Src0->getType());
-return CGF.Builder.CreateConstrainedFPCall(F, {Src0, Src1}, "",
- std::nullopt, std::nullopt,
&FMF);
+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}, "", nullptr, &FMF);
``
https://github.com/llvm/llvm-project/pull/113133
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] Clang: Add nsz to llvm.minnum and llvm.maxnum emitted from fmin and fmax (PR #113133)
@@ -602,19 +602,20 @@ 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) {
arsenm wrote:
```suggestion
unsigned ConstrainedIntrinsicID, llvm::FastMathFlags FMF = {}) {
```
https://github.com/llvm/llvm-project/pull/113133
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] Clang: Add nsz to llvm.minnum and llvm.maxnum emitted from fmin and fmax (PR #113133)
arsenm wrote: > Do you have any data on the impact of the change? This change in isolation shouldn't change anything. In the absence of changing the minnum definition to have strict zero handling, adding nsz is a no-op. With strict signed zero handling, this would help avoid regressions in the lowering https://github.com/llvm/llvm-project/pull/113133 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] Clang: Add nsz to llvm.minnum and llvm.maxnum emitted from fmin and fmax (PR #113133)
wzssyqa wrote: > Do you have any data on the impact of the change? It is not used yet. We will use it in `TargetLowering::expandFMINNUM_FMAXNUM`. https://github.com/llvm/llvm-project/pull/113133 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] Clang: Add nsz to llvm.minnum and llvm.maxnum emitted from fmin and fmax (PR #113133)
https://github.com/fhahn commented: Do you have any data on the impact of the change? https://github.com/llvm/llvm-project/pull/113133 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] Clang: Add nsz to llvm.minnum and llvm.maxnum emitted from fmin and fmax (PR #113133)
wzssyqa wrote: @arsenm ping https://github.com/llvm/llvm-project/pull/113133 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] Clang: Add nsz to llvm.minnum and llvm.maxnum emitted from fmin and fmax (PR #113133)
arsenm wrote: > #168838 > > OK, now we decides to define `llvm.minnum` and `llvm.maxnum` as same as libm. > So we don't need it now. Let's close it. No, this should still be done https://github.com/llvm/llvm-project/pull/113133 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] Clang: Add nsz to llvm.minnum and llvm.maxnum emitted from fmin and fmax (PR #113133)
https://github.com/arsenm reopened https://github.com/llvm/llvm-project/pull/113133 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] Clang: Add nsz to llvm.minnum and llvm.maxnum emitted from fmin and fmax (PR #113133)
https://github.com/wzssyqa closed https://github.com/llvm/llvm-project/pull/113133 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] Clang: Add nsz to llvm.minnum and llvm.maxnum emitted from fmin and fmax (PR #113133)
wzssyqa wrote: https://github.com/llvm/llvm-project/pull/168838 OK, now we decides to define `llvm.minnum` and `llvm.maxnum` as same as libm. So we don't need it now. Let's close it. https://github.com/llvm/llvm-project/pull/113133 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] Clang: Add nsz to llvm.minnum and llvm.maxnum emitted from fmin and fmax (PR #113133)
wzssyqa wrote: > It takes too long to take this decision. Since we cannot agree on the wording > of `nsz` now, and in fact we had some words in `maxnum/minnum` about nsz. I > prefer we can continue with this PR. We have got some complains: https://github.com/llvm/llvm-project/issues/138303 https://github.com/llvm/llvm-project/pull/113133 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] Clang: Add nsz to llvm.minnum and llvm.maxnum emitted from fmin and fmax (PR #113133)
wzssyqa wrote: It takes too long to take this decision. Since we cannot agree on the wording of `nsz` now, and in fact we had some words in `maxnum/minnum` about nsz. I prefer we can continue with this PR. https://github.com/llvm/llvm-project/pull/113133 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] Clang: Add nsz to llvm.minnum and llvm.maxnum emitted from fmin and fmax (PR #113133)
efriedma-quic wrote: 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. https://github.com/llvm/llvm-project/pull/113133 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] Clang: Add nsz to llvm.minnum and llvm.maxnum emitted from fmin and fmax (PR #113133)
wzssyqa wrote: > > > 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. ``` https://github.com/llvm/llvm-project/pull/113133 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] Clang: Add nsz to llvm.minnum and llvm.maxnum emitted from fmin and fmax (PR #113133)
nikic wrote: > > 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... https://github.com/llvm/llvm-project/pull/113133 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] Clang: Add nsz to llvm.minnum and llvm.maxnum emitted from fmin and fmax (PR #113133)
nikic wrote: 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. https://github.com/llvm/llvm-project/pull/113133 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] Clang: Add nsz to llvm.minnum and llvm.maxnum emitted from fmin and fmax (PR #113133)
arsenm wrote: > 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 https://github.com/llvm/llvm-project/pull/113133 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] Clang: Add nsz to llvm.minnum and llvm.maxnum emitted from fmin and fmax (PR #113133)
wzssyqa wrote: ping https://github.com/llvm/llvm-project/pull/113133 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
