[llvm] [Float2Int] Fix miscompile with floats that can be converted to large values (PR #85996)
via llvm-commits
llvm-commits at lists.llvm.org
Wed Mar 20 15:12:06 PDT 2024
https://github.com/AtariDreams updated https://github.com/llvm/llvm-project/pull/85996
>From 06e90c8d074b5cc51447a664ad4b24d8e39f2594 Mon Sep 17 00:00:00 2001
From: Rose <gfunni234 at gmail.com>
Date: Wed, 20 Mar 2024 13:24:30 -0400
Subject: [PATCH 1/3] [Float2Int] Pre-commit tests (NFC)
---
llvm/test/Transforms/Float2Int/basic.ll | 52 +++++++++++++++++++++++++
1 file changed, 52 insertions(+)
diff --git a/llvm/test/Transforms/Float2Int/basic.ll b/llvm/test/Transforms/Float2Int/basic.ll
index 2854a83179b7eb..e4c2f5c983c7e2 100644
--- a/llvm/test/Transforms/Float2Int/basic.ll
+++ b/llvm/test/Transforms/Float2Int/basic.ll
@@ -349,3 +349,55 @@ bogusBB: ; preds = %bogusBB
%tobool = fcmp une double %inc, 0.000000e+00
br label %bogusBB
}
+
+define i32 @pr79158() {
+; CHECK-LABEL: @pr79158(
+; CHECK-NEXT: entry:
+; CHECK-NEXT: [[X_I:%.*]] = alloca i32, align 4
+; CHECK-NEXT: call void @llvm.lifetime.start.p0(i64 4, ptr nonnull [[X_I]])
+; CHECK-NEXT: store volatile i32 1, ptr [[X_I]], align 4
+; CHECK-NEXT: [[X_I_0_X_I_0_X_0_X_0_X_0__I:%.*]] = load volatile i32, ptr [[X_I]], align 4
+; CHECK-NEXT: [[CMP_I:%.*]] = icmp sgt i32 [[X_I_0_X_I_0_X_0_X_0_X_0__I]], 0
+; CHECK-NEXT: [[TMP0:%.*]] = zext i1 [[CMP_I]] to i32
+; CHECK-NEXT: [[MUL_I1:%.*]] = mul i32 [[TMP0]], 2147483647
+; CHECK-NEXT: call void @llvm.lifetime.end.p0(i64 4, ptr nonnull [[X_I]])
+; CHECK-NEXT: ret i32 0
+;
+entry:
+ %x.i = alloca i32, align 4
+ call void @llvm.lifetime.start.p0(i64 4, ptr nonnull %x.i)
+ store volatile i32 1, ptr %x.i, align 4
+ %x.i.0.x.i.0.x.0.x.0.x.0..i = load volatile i32, ptr %x.i, align 4
+ %cmp.i = icmp sgt i32 %x.i.0.x.i.0.x.0.x.0.x.0..i, 0
+ %conv.i = uitofp i1 %cmp.i to double
+ %mul.i = fmul double %conv.i, 4294967295.0
+ %conv1.i = fptoui double %mul.i to i32
+ call void @llvm.lifetime.end.p0(i64 4, ptr nonnull %x.i)
+ ret i32 0
+}
+
+define i32 @pr79158_2() {
+; CHECK-LABEL: @pr79158_2(
+; CHECK-NEXT: entry:
+; CHECK-NEXT: [[X_I:%.*]] = alloca i32, align 4
+; CHECK-NEXT: call void @llvm.lifetime.start.p0(i64 4, ptr nonnull [[X_I]])
+; CHECK-NEXT: store volatile i32 1, ptr [[X_I]], align 4
+; CHECK-NEXT: [[X_I_0_X_I_0_X_0_X_0_X_0__I:%.*]] = load volatile i32, ptr [[X_I]], align 4
+; CHECK-NEXT: [[CMP_I:%.*]] = icmp sgt i32 [[X_I_0_X_I_0_X_0_X_0_X_0__I]], 0
+; CHECK-NEXT: [[TMP0:%.*]] = zext i1 [[CMP_I]] to i32
+; CHECK-NEXT: [[MUL_I1:%.*]] = mul i32 [[TMP0]], 2147483647
+; CHECK-NEXT: call void @llvm.lifetime.end.p0(i64 4, ptr nonnull [[X_I]])
+; CHECK-NEXT: ret i32 0
+;
+entry:
+ %x.i = alloca i32, align 4
+ call void @llvm.lifetime.start.p0(i64 4, ptr nonnull %x.i)
+ store volatile i32 1, ptr %x.i, align 4
+ %x.i.0.x.i.0.x.0.x.0.x.0..i = load volatile i32, ptr %x.i, align 4
+ %cmp.i = icmp sgt i32 %x.i.0.x.i.0.x.0.x.0.x.0..i, 0
+ %conv.i = uitofp i1 %cmp.i to double
+ %mul.i = fmul double %conv.i, 2147483648.0
+ %conv1.i = fptoui double %mul.i to i32
+ call void @llvm.lifetime.end.p0(i64 4, ptr nonnull %x.i)
+ ret i32 0
+}
>From 1633bc30d8f74b437dc54499cdedc9c690e73fb3 Mon Sep 17 00:00:00 2001
From: Rose <gfunni234 at gmail.com>
Date: Wed, 20 Mar 2024 15:34:39 -0400
Subject: [PATCH 2/3] [Float2Int] Fix miscompile with floats that can be
converted to large values
There is an issue with Float2Int whenever it has to deal with floats that evaluate to numbers that are over INT_MAX, but under UINT_MAX.
Simply, because the conversion method always assumes a signed integer, this works until the 32 bit number is actually a positive 32 bit number that would be interpreted as a negative number when evaluated as an integer, being put in an i32.
This was messing up the Float2Int conversions and causing miscompiles.
To fix this, tell the APSInt constructor if the value is positive or negative.
---
llvm/lib/Transforms/Scalar/Float2Int.cpp | 7 +++----
llvm/test/Transforms/Float2Int/basic.ll | 4 ++--
2 files changed, 5 insertions(+), 6 deletions(-)
diff --git a/llvm/lib/Transforms/Scalar/Float2Int.cpp b/llvm/lib/Transforms/Scalar/Float2Int.cpp
index ccca8bcc1a56ac..2805a9adeaec5e 100644
--- a/llvm/lib/Transforms/Scalar/Float2Int.cpp
+++ b/llvm/lib/Transforms/Scalar/Float2Int.cpp
@@ -409,11 +409,10 @@ Value *Float2IntPass::convert(Instruction *I, Type *ToTy) {
} else if (Instruction *VI = dyn_cast<Instruction>(V)) {
NewOperands.push_back(convert(VI, ToTy));
} else if (ConstantFP *CF = dyn_cast<ConstantFP>(V)) {
- APSInt Val(ToTy->getPrimitiveSizeInBits(), /*isUnsigned=*/false);
+ const APFloat &F = CF->getValueAPF();
+ APSInt Val(ToTy->getPrimitiveSizeInBits(), !F.isNegative());
bool Exact;
- CF->getValueAPF().convertToInteger(Val,
- APFloat::rmNearestTiesToEven,
- &Exact);
+ F.convertToInteger(Val, APFloat::rmNearestTiesToEven, &Exact);
NewOperands.push_back(ConstantInt::get(ToTy, Val));
} else {
llvm_unreachable("Unhandled operand type?");
diff --git a/llvm/test/Transforms/Float2Int/basic.ll b/llvm/test/Transforms/Float2Int/basic.ll
index e4c2f5c983c7e2..73659d4dec4528 100644
--- a/llvm/test/Transforms/Float2Int/basic.ll
+++ b/llvm/test/Transforms/Float2Int/basic.ll
@@ -359,7 +359,7 @@ define i32 @pr79158() {
; CHECK-NEXT: [[X_I_0_X_I_0_X_0_X_0_X_0__I:%.*]] = load volatile i32, ptr [[X_I]], align 4
; CHECK-NEXT: [[CMP_I:%.*]] = icmp sgt i32 [[X_I_0_X_I_0_X_0_X_0_X_0__I]], 0
; CHECK-NEXT: [[TMP0:%.*]] = zext i1 [[CMP_I]] to i32
-; CHECK-NEXT: [[MUL_I1:%.*]] = mul i32 [[TMP0]], 2147483647
+; CHECK-NEXT: [[MUL_I1:%.*]] = mul i32 [[TMP0]], -1
; CHECK-NEXT: call void @llvm.lifetime.end.p0(i64 4, ptr nonnull [[X_I]])
; CHECK-NEXT: ret i32 0
;
@@ -385,7 +385,7 @@ define i32 @pr79158_2() {
; CHECK-NEXT: [[X_I_0_X_I_0_X_0_X_0_X_0__I:%.*]] = load volatile i32, ptr [[X_I]], align 4
; CHECK-NEXT: [[CMP_I:%.*]] = icmp sgt i32 [[X_I_0_X_I_0_X_0_X_0_X_0__I]], 0
; CHECK-NEXT: [[TMP0:%.*]] = zext i1 [[CMP_I]] to i32
-; CHECK-NEXT: [[MUL_I1:%.*]] = mul i32 [[TMP0]], 2147483647
+; CHECK-NEXT: [[MUL_I1:%.*]] = mul i32 [[TMP0]], -2147483648
; CHECK-NEXT: call void @llvm.lifetime.end.p0(i64 4, ptr nonnull [[X_I]])
; CHECK-NEXT: ret i32 0
;
>From 4c8b8d295dfb97e512f8a273d1c304d6a9338c12 Mon Sep 17 00:00:00 2001
From: Rose <gfunni234 at gmail.com>
Date: Wed, 20 Mar 2024 18:11:34 -0400
Subject: [PATCH 3/3] Fix range
---
llvm/lib/Transforms/Scalar/Float2Int.cpp | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/llvm/lib/Transforms/Scalar/Float2Int.cpp b/llvm/lib/Transforms/Scalar/Float2Int.cpp
index 2805a9adeaec5e..0cc782cb80bbc9 100644
--- a/llvm/lib/Transforms/Scalar/Float2Int.cpp
+++ b/llvm/lib/Transforms/Scalar/Float2Int.cpp
@@ -358,10 +358,9 @@ bool Float2IntPass::validateAndTransform() {
assert(ConvertedToTy && "Must have set the convertedtoty by this point!");
// The number of bits required is the maximum of the upper and
- // lower limits, plus one so it can be signed.
+ // lower limits.
unsigned MinBW = std::max(R.getLower().getSignificantBits(),
- R.getUpper().getSignificantBits()) +
- 1;
+ R.getUpper().getSignificantBits());
LLVM_DEBUG(dbgs() << "F2I: MinBitwidth=" << MinBW << ", R: " << R << "\n");
// If we've run off the realms of the exactly representable integers,
@@ -410,7 +409,7 @@ Value *Float2IntPass::convert(Instruction *I, Type *ToTy) {
NewOperands.push_back(convert(VI, ToTy));
} else if (ConstantFP *CF = dyn_cast<ConstantFP>(V)) {
const APFloat &F = CF->getValueAPF();
- APSInt Val(ToTy->getPrimitiveSizeInBits(), !F.isNegative());
+ APSInt Val(ToTy->getPrimitiveSizeInBits(), false); //! F.isNegative());
bool Exact;
F.convertToInteger(Val, APFloat::rmNearestTiesToEven, &Exact);
NewOperands.push_back(ConstantInt::get(ToTy, Val));
More information about the llvm-commits
mailing list