[llvm] [Float2Int] Fix miscompile with floats that can be converted to large values (PR #85996)

via llvm-commits llvm-commits at lists.llvm.org
Thu Mar 21 11:45:54 PDT 2024


https://github.com/AtariDreams updated https://github.com/llvm/llvm-project/pull/85996

>From 7198019a34378ce2572f6f27a090f3eb815569a8 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/2] [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..f3d749b60e162c 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 [[MUL_I1]]
+;
+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 %conv1.i
+}
+
+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 [[MUL_I1]]
+;
+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 %conv1.i
+}

>From 8bd91abe4619cf57e2646fb1fd80e1f22cfa87fe 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/2] [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 de8c05d5689f5b..7e862882310b85 100644
--- a/llvm/lib/Transforms/Scalar/Float2Int.cpp
+++ b/llvm/lib/Transforms/Scalar/Float2Int.cpp
@@ -407,11 +407,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 f3d749b60e162c..4c118a4e55f432 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 [[MUL_I1]]
 ;
@@ -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 [[MUL_I1]]
 ;



More information about the llvm-commits mailing list