[llvm] 83ba349 - [InstSimplify] fix/improve folding with an SNaN operand
Sanjay Patel via llvm-commits
llvm-commits at lists.llvm.org
Tue Feb 14 14:51:15 PST 2023
Author: Sanjay Patel
Date: 2023-02-14T17:51:06-05:00
New Revision: 83ba349ae0a853e0c2cd8e8aadc88993e9fb9a19
URL: https://github.com/llvm/llvm-project/commit/83ba349ae0a853e0c2cd8e8aadc88993e9fb9a19
DIFF: https://github.com/llvm/llvm-project/commit/83ba349ae0a853e0c2cd8e8aadc88993e9fb9a19.diff
LOG: [InstSimplify] fix/improve folding with an SNaN operand
There are 2 issues here:
1. In the default LLVM FP environment (regular FP math instructions),
SNaN is some flavor of "don't care" which we will nail down in
D143074, so this is just a quality-of-implementation improvement
for default FP.
2. In the constrained FP environment (constrained intrinsics), SNaN
must not propagate through a math operation; it has to be quieted
according to IEEE-754 spec. That is independent of exception
handling mode, so the current behavior is a miscompile.
Differential Revision: https://reviews.llvm.org/D143505
Added:
Modified:
llvm/include/llvm/ADT/APFloat.h
llvm/lib/Analysis/InstructionSimplify.cpp
llvm/lib/Target/AMDGPU/AMDGPUInstCombineIntrinsic.cpp
llvm/test/Transforms/InstSimplify/fp-nan.ll
llvm/test/Transforms/InstSimplify/strictfp-fadd.ll
Removed:
################################################################################
diff --git a/llvm/include/llvm/ADT/APFloat.h b/llvm/include/llvm/ADT/APFloat.h
index 402ffba3ff8e4..38238504d4d9f 100644
--- a/llvm/include/llvm/ADT/APFloat.h
+++ b/llvm/include/llvm/ADT/APFloat.h
@@ -1136,6 +1136,14 @@ class APFloat : public APFloatBase {
return Value;
}
+ /// Assuming this is an IEEE-754 NaN value, quiet its signaling bit.
+ /// This preserves the sign and payload bits.
+ APFloat makeQuiet() const {
+ APFloat Result(*this);
+ Result.getIEEE().makeQuiet();
+ return Result;
+ }
+
opStatus convert(const fltSemantics &ToSemantics, roundingMode RM,
bool *losesInfo);
opStatus convertToInteger(MutableArrayRef<integerPart> Input,
diff --git a/llvm/lib/Analysis/InstructionSimplify.cpp b/llvm/lib/Analysis/InstructionSimplify.cpp
index 4ca117b6043ed..c71ff9474bd9b 100644
--- a/llvm/lib/Analysis/InstructionSimplify.cpp
+++ b/llvm/lib/Analysis/InstructionSimplify.cpp
@@ -5309,13 +5309,15 @@ Value *llvm::simplifyFNegInst(Value *Op, FastMathFlags FMF,
/// Try to propagate existing NaN values when possible. If not, replace the
/// constant or elements in the constant with a canonical NaN.
static Constant *propagateNaN(Constant *In) {
- if (auto *VecTy = dyn_cast<FixedVectorType>(In->getType())) {
+ Type *Ty = In->getType();
+ if (auto *VecTy = dyn_cast<FixedVectorType>(Ty)) {
unsigned NumElts = VecTy->getNumElements();
SmallVector<Constant *, 32> NewC(NumElts);
for (unsigned i = 0; i != NumElts; ++i) {
Constant *EltC = In->getAggregateElement(i);
// Poison and existing NaN elements propagate.
// Replace unknown or undef elements with canonical NaN.
+ // TODO: Quiet a signaling NaN element.
if (EltC && (isa<PoisonValue>(EltC) || EltC->isNaN()))
NewC[i] = EltC;
else
@@ -5324,13 +5326,14 @@ static Constant *propagateNaN(Constant *In) {
return ConstantVector::get(NewC);
}
- // It is not a fixed vector, but not a simple NaN either?
+ // If it is not a fixed vector, but not a simple NaN either, return a
+ // canonical NaN.
if (!In->isNaN())
- return ConstantFP::getNaN(In->getType());
+ return ConstantFP::getNaN(Ty);
- // Propagate the existing NaN constant when possible.
- // TODO: Should we quiet a signaling NaN?
- return In;
+ // Propagate an existing QNaN constant. If it is an SNaN, make it quiet, but
+ // preserve the sign/payload.
+ return ConstantFP::get(Ty, cast<ConstantFP>(In)->getValue().makeQuiet());
}
/// Perform folds that are common to any floating-point operation. This implies
diff --git a/llvm/lib/Target/AMDGPU/AMDGPUInstCombineIntrinsic.cpp b/llvm/lib/Target/AMDGPU/AMDGPUInstCombineIntrinsic.cpp
index 62c3eec41836e..a50511ef3e6df 100644
--- a/llvm/lib/Target/AMDGPU/AMDGPUInstCombineIntrinsic.cpp
+++ b/llvm/lib/Target/AMDGPU/AMDGPUInstCombineIntrinsic.cpp
@@ -981,13 +981,8 @@ GCNTTIImpl::instCombineIntrinsic(InstCombiner &IC, IntrinsicInst &II) const {
if (II.isStrictFP())
break;
- if (C && C->isNaN()) {
- // FIXME: We just need to make the nan quiet here, but that's unavailable
- // on APFloat, only IEEEfloat
- auto *Quieted =
- ConstantFP::get(Ty, scalbn(*C, 0, APFloat::rmNearestTiesToEven));
- return IC.replaceInstUsesWith(II, Quieted);
- }
+ if (C && C->isNaN())
+ return IC.replaceInstUsesWith(II, ConstantFP::get(Ty, C->makeQuiet()));
// ldexp(x, 0) -> x
// ldexp(x, undef) -> x
diff --git a/llvm/test/Transforms/InstSimplify/fp-nan.ll b/llvm/test/Transforms/InstSimplify/fp-nan.ll
index 1a2797272c093..25d3a0affed8c 100644
--- a/llvm/test/Transforms/InstSimplify/fp-nan.ll
+++ b/llvm/test/Transforms/InstSimplify/fp-nan.ll
@@ -31,21 +31,21 @@ define float @fsub_nan_op0(float %x) {
ret float %r
}
-; Signaling
+; Signaling - make quiet and preserve the payload and signbit
define float @fsub_nan_op1(float %x) {
; CHECK-LABEL: @fsub_nan_op1(
-; CHECK-NEXT: ret float 0x7FF1000000000000
+; CHECK-NEXT: ret float 0x7FF9000000000000
;
%r = fsub float %x, 0x7FF1000000000000
ret float %r
}
-; Signaling and signed
+; Signaling and signed - make quiet and preserve the payload and signbit
define double @fmul_nan_op0(double %x) {
; CHECK-LABEL: @fmul_nan_op0(
-; CHECK-NEXT: ret double 0xFFF0000000000001
+; CHECK-NEXT: ret double 0xFFF8000000000001
;
%r = fmul double 0xFFF0000000000001, %x
ret double %r
diff --git a/llvm/test/Transforms/InstSimplify/strictfp-fadd.ll b/llvm/test/Transforms/InstSimplify/strictfp-fadd.ll
index f36b6180930cd..f27e430c68662 100644
--- a/llvm/test/Transforms/InstSimplify/strictfp-fadd.ll
+++ b/llvm/test/Transforms/InstSimplify/strictfp-fadd.ll
@@ -370,17 +370,21 @@ define float @fold_fadd_qnan_qnan_ebstrict() #0 {
ret float %add
}
+; Exceptions are ignored, so this can be folded, but constrained math requires that SNaN is quieted per IEEE-754 spec.
+
define float @fold_fadd_snan_variable_ebignore(float %x) #0 {
; CHECK-LABEL: @fold_fadd_snan_variable_ebignore(
-; CHECK-NEXT: ret float 0x7FF4000000000000
+; CHECK-NEXT: ret float 0x7FFC000000000000
;
%add = call float @llvm.experimental.constrained.fadd.f32(float 0x7ff4000000000000, float %x, metadata !"round.tonearest", metadata !"fpexcept.ignore") #0
ret float %add
}
+; Exceptions may (not) trap, so this can be folded, but constrained math requires that SNaN is quieted per IEEE-754 spec.
+
define float @fold_fadd_snan_variable_ebmaytrap(float %x) #0 {
; CHECK-LABEL: @fold_fadd_snan_variable_ebmaytrap(
-; CHECK-NEXT: ret float 0x7FF4000000000000
+; CHECK-NEXT: ret float 0x7FFC000000000000
;
%add = call float @llvm.experimental.constrained.fadd.f32(float 0x7ff4000000000000, float %x, metadata !"round.tonearest", metadata !"fpexcept.maytrap") #0
ret float %add
More information about the llvm-commits
mailing list