Skip to content

Commit 823db1f

Browse files
authored
Merge pull request #71610 from eeckstein/fix-const-folding
ConstantFolding: fix wrong constant folding for float-infinity compares
2 parents ecf4256 + fa85745 commit 823db1f

File tree

3 files changed

+64
-22
lines changed

3 files changed

+64
-22
lines changed

lib/SILOptimizer/Utils/ConstantFolding.cpp

+17-22
Original file line numberDiff line numberDiff line change
@@ -381,6 +381,13 @@ static SILValue constantFoldIntrinsic(BuiltinInst *BI, llvm::Intrinsic::ID ID,
381381
return nullptr;
382382
}
383383

384+
static bool isFiniteFloatLiteral(SILValue v) {
385+
if (auto *lit = dyn_cast<FloatLiteralInst>(v)) {
386+
return lit->getValue().isFinite();
387+
}
388+
return false;
389+
}
390+
384391
static SILValue constantFoldCompareFloat(BuiltinInst *BI, BuiltinValueKind ID) {
385392
static auto hasIEEEFloatNanBitRepr = [](const APInt val) -> bool {
386393
auto bitWidth = val.getBitWidth();
@@ -640,17 +647,11 @@ static SILValue constantFoldCompareFloat(BuiltinInst *BI, BuiltinValueKind ID) {
640647
m_BuiltinInst(BuiltinValueKind::FCMP_ULE,
641648
m_SILValue(Other), m_BitCast(m_IntegerLiteralInst(builtinArg)))))) {
642649
APInt val = builtinArg->getValue();
643-
if (hasIEEEFloatPosInfBitRepr(val)) {
644-
// One of the operands is infinity, but unless the other operand is not
645-
// fully visible we cannot definitively say what it is. It can be anything,
646-
// including NaN and infinity itself. Therefore, we cannot fold the comparison
647-
// just yet.
648-
if (isa<StructExtractInst>(Other) || isa<TupleExtractInst>(Other)) {
649-
return nullptr;
650-
} else {
651-
SILBuilderWithScope B(BI);
652-
return B.createIntegerLiteral(BI->getLoc(), BI->getType(), APInt(1, 1));
653-
}
650+
if (hasIEEEFloatPosInfBitRepr(val) &&
651+
// Only if `Other` is a literal we can be sure that it's not Inf or NaN.
652+
isFiniteFloatLiteral(Other)) {
653+
SILBuilderWithScope B(BI);
654+
return B.createIntegerLiteral(BI->getLoc(), BI->getType(), APInt(1, 1));
654655
}
655656
}
656657

@@ -682,17 +683,11 @@ static SILValue constantFoldCompareFloat(BuiltinInst *BI, BuiltinValueKind ID) {
682683
m_BuiltinInst(BuiltinValueKind::FCMP_ULE,
683684
m_BitCast(m_IntegerLiteralInst(builtinArg)), m_SILValue(Other))))) {
684685
APInt val = builtinArg->getValue();
685-
if (hasIEEEFloatPosInfBitRepr(val)) {
686-
// One of the operands is infinity, but unless the other operand is not
687-
// fully visible we cannot definitively say what it is. It can be anything,
688-
// including NaN and infinity itself. Therefore, we cannot fold the comparison
689-
// just yet.
690-
if (isa<StructExtractInst>(Other) || isa<TupleExtractInst>(Other)) {
691-
return nullptr;
692-
} else {
693-
SILBuilderWithScope B(BI);
694-
return B.createIntegerLiteral(BI->getLoc(), BI->getType(), APInt(1, 0));
695-
}
686+
if (hasIEEEFloatPosInfBitRepr(val) &&
687+
// Only if `Other` is a literal we can be sure that it's not Inf or NaN.
688+
isFiniteFloatLiteral(Other)) {
689+
SILBuilderWithScope B(BI);
690+
return B.createIntegerLiteral(BI->getLoc(), BI->getType(), APInt(1, 0));
696691
}
697692
}
698693

Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
// RUN: %target-swift-frontend -parse-as-library -module-name test %s -O -emit-sil | %FileCheck %s
2+
3+
// REQUIRES: swift_stdlib_no_asserts,optimized_stdlib
4+
5+
// CHECK-LABEL: sil @$s4test17dont_fold_inf_cmpySbSfF :
6+
// CHECK: builtin "fcmp_olt_FPIEEE32"
7+
// CHECK: } // end sil function '$s4test17dont_fold_inf_cmpySbSfF'
8+
public func dont_fold_inf_cmp(_ f: Float) -> Bool {
9+
(f + 0) < .infinity
10+
}
11+
12+
// CHECK-LABEL: sil @$s4test014dont_fold_inf_D4_cmpSbyF :
13+
// CHECK: builtin "fcmp_olt_FPIEEE32"
14+
// CHECK: } // end sil function '$s4test014dont_fold_inf_D4_cmpSbyF'
15+
public func dont_fold_inf_inf_cmp() -> Bool {
16+
0x1.0p128 < Float.infinity
17+
}
18+

test/SILOptimizer/constant_propagation.sil

+29
Original file line numberDiff line numberDiff line change
@@ -870,6 +870,35 @@ bb0:
870870
// CHECK-NEXT: } // end sil function 'fold_double_comparison_with_inf'
871871
}
872872

873+
sil @dont_fold_comparison_with_inf : $@convention(thin) (Builtin.FPIEEE32) -> Builtin.Int1 {
874+
bb0(%0 : $Builtin.FPIEEE32):
875+
%2 = float_literal $Builtin.FPIEEE32, 0x0
876+
%4 = builtin "fadd_FPIEEE32"(%0 : $Builtin.FPIEEE32, %2 : $Builtin.FPIEEE32) : $Builtin.FPIEEE32
877+
%5 = integer_literal $Builtin.Int32, 2139095040
878+
%6 = builtin "bitcast_Int32_FPIEEE32"(%5 : $Builtin.Int32) : $Builtin.FPIEEE32
879+
%9 = builtin "fcmp_olt_FPIEEE32"(%4 : $Builtin.FPIEEE32, %6 : $Builtin.FPIEEE32) : $Builtin.Int1
880+
return %9 : $Builtin.Int1
881+
882+
// CHECK-LABEL: sil @dont_fold_comparison_with_inf :
883+
// CHECK: [[R:%.*]] = builtin "fcmp_olt_FPIEEE32"
884+
// CHECK: return [[R]]
885+
// CHECK: } // end sil function 'dont_fold_comparison_with_inf'
886+
}
887+
888+
sil @dont_fold_comparison_with_inf2 : $@convention(thin) () -> Builtin.Int1 {
889+
bb0:
890+
%2 = float_literal $Builtin.FPIEEE32, 0x7F800000 // +Inf // user: %3
891+
%5 = integer_literal $Builtin.Int32, 2139095040
892+
%6 = builtin "bitcast_Int32_FPIEEE32"(%5 : $Builtin.Int32) : $Builtin.FPIEEE32
893+
%9 = builtin "fcmp_olt_FPIEEE32"(%2 : $Builtin.FPIEEE32, %6 : $Builtin.FPIEEE32) : $Builtin.Int1
894+
return %9 : $Builtin.Int1
895+
896+
// CHECK-LABEL: sil @dont_fold_comparison_with_inf2 :
897+
// CHECK: [[R:%.*]] = builtin "fcmp_olt_FPIEEE32"
898+
// CHECK: return [[R]]
899+
// CHECK: } // end sil function 'dont_fold_comparison_with_inf2'
900+
}
901+
873902
// fold float comparison operations with Infinity/NaN when the other argument is not constant
874903
sil @fold_float_comparison_with_non_constant_arg : $@convention(thin) (Float) -> () {
875904
bb0(%0: $Float):

0 commit comments

Comments
 (0)