[llvm] [ConstantRange] Fix off by 1 bugs in UIToFP and SIToFP handling. (PR #86041)
Craig Topper via llvm-commits
llvm-commits at lists.llvm.org
Wed Mar 20 17:09:40 PDT 2024
https://github.com/topperc updated https://github.com/llvm/llvm-project/pull/86041
>From 05601500bc721e6c4d0eb52b309e955efd33e111 Mon Sep 17 00:00:00 2001
From: Craig Topper <craig.topper at sifive.com>
Date: Wed, 20 Mar 2024 17:05:17 -0700
Subject: [PATCH 1/2] [Float2Int] Pre-commit test for ConstantRange bug. NFC
---
llvm/test/Transforms/Float2Int/pr79158.ll | 19 +++++++++++++++++++
1 file changed, 19 insertions(+)
create mode 100644 llvm/test/Transforms/Float2Int/pr79158.ll
diff --git a/llvm/test/Transforms/Float2Int/pr79158.ll b/llvm/test/Transforms/Float2Int/pr79158.ll
new file mode 100644
index 00000000000000..ec7d437f895014
--- /dev/null
+++ b/llvm/test/Transforms/Float2Int/pr79158.ll
@@ -0,0 +1,19 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 4
+; RUN: opt < %s -passes=float2int -S | FileCheck %s
+
+define i32 @pr79158(i32 %x.i.0.x.i.0.x.0.x.0.x.0..i) {
+; CHECK-LABEL: define i32 @pr79158(
+; CHECK-SAME: i32 [[X_I_0_X_I_0_X_0_X_0_X_0__I:%.*]]) {
+; CHECK-NEXT: entry:
+; 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: ret i32 [[MUL_I1]]
+;
+entry:
+ %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, 0x41EFFFFFFFE00000
+ %conv1.i = fptoui double %mul.i to i32
+ ret i32 %conv1.i
+}
>From e3c5053838933b8d5729825819626f2f04e19b6e Mon Sep 17 00:00:00 2001
From: Craig Topper <craig.topper at sifive.com>
Date: Wed, 20 Mar 2024 16:36:44 -0700
Subject: [PATCH 2/2] [ConstantRange] Fix off by 1 bugs in UIToFP and SIToFP
handling.
We were passing the min and max values of the range to the ConstantRange
constructor, but the constructor expects the upper bound to 1
more than the max value so we need to add 1.
We also need to use getNonEmpty so that passing 0, 0 to the constructor
creates a full range rather than an empty range. And passing smin, smax+1
doesn't cause an assertion.
I believe this fixes at least some of the reason #79158 was reverted.
---
llvm/lib/IR/ConstantRange.cpp | 4 ++--
llvm/test/Transforms/Float2Int/pr79158.ll | 5 +++--
llvm/unittests/IR/ConstantRangeTest.cpp | 18 ++++++++++++++++++
3 files changed, 23 insertions(+), 4 deletions(-)
diff --git a/llvm/lib/IR/ConstantRange.cpp b/llvm/lib/IR/ConstantRange.cpp
index 3394a1ec8dc476..59e7a9f5eb1114 100644
--- a/llvm/lib/IR/ConstantRange.cpp
+++ b/llvm/lib/IR/ConstantRange.cpp
@@ -746,7 +746,7 @@ ConstantRange ConstantRange::castOp(Instruction::CastOps CastOp,
Min = Min.zext(ResultBitWidth);
Max = Max.zext(ResultBitWidth);
}
- return ConstantRange(std::move(Min), std::move(Max));
+ return getNonEmpty(std::move(Min), std::move(Max) + 1);
}
case Instruction::SIToFP: {
// TODO: use input range if available
@@ -757,7 +757,7 @@ ConstantRange ConstantRange::castOp(Instruction::CastOps CastOp,
SMin = SMin.sext(ResultBitWidth);
SMax = SMax.sext(ResultBitWidth);
}
- return ConstantRange(std::move(SMin), std::move(SMax));
+ return getNonEmpty(std::move(SMin), std::move(SMax) + 1);
}
case Instruction::FPTrunc:
case Instruction::FPExt:
diff --git a/llvm/test/Transforms/Float2Int/pr79158.ll b/llvm/test/Transforms/Float2Int/pr79158.ll
index ec7d437f895014..88b94c8cdb02cc 100644
--- a/llvm/test/Transforms/Float2Int/pr79158.ll
+++ b/llvm/test/Transforms/Float2Int/pr79158.ll
@@ -6,8 +6,9 @@ define i32 @pr79158(i32 %x.i.0.x.i.0.x.0.x.0.x.0..i) {
; CHECK-SAME: i32 [[X_I_0_X_I_0_X_0_X_0_X_0__I:%.*]]) {
; CHECK-NEXT: entry:
; 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: [[TMP0:%.*]] = zext i1 [[CMP_I]] to i64
+; CHECK-NEXT: [[MUL_I2:%.*]] = mul i64 [[TMP0]], 4294967295
+; CHECK-NEXT: [[MUL_I1:%.*]] = trunc i64 [[MUL_I2]] to i32
; CHECK-NEXT: ret i32 [[MUL_I1]]
;
entry:
diff --git a/llvm/unittests/IR/ConstantRangeTest.cpp b/llvm/unittests/IR/ConstantRangeTest.cpp
index 34a162a5514e95..8ec120d70e99f6 100644
--- a/llvm/unittests/IR/ConstantRangeTest.cpp
+++ b/llvm/unittests/IR/ConstantRangeTest.cpp
@@ -2479,6 +2479,24 @@ TEST_F(ConstantRangeTest, castOps) {
ConstantRange IntToPtr = A.castOp(Instruction::IntToPtr, 64);
EXPECT_EQ(64u, IntToPtr.getBitWidth());
EXPECT_TRUE(IntToPtr.isFullSet());
+
+ ConstantRange UIToFP = A.castOp(Instruction::UIToFP, 16);
+ EXPECT_EQ(16u, UIToFP.getBitWidth());
+ EXPECT_TRUE(UIToFP.isFullSet());
+
+ ConstantRange UIToFP2 = A.castOp(Instruction::UIToFP, 64);
+ ConstantRange B(APInt(64, 0), APInt(64, 65536));
+ EXPECT_EQ(64u, UIToFP2.getBitWidth());
+ EXPECT_EQ(B, UIToFP2);
+
+ ConstantRange SIToFP = A.castOp(Instruction::SIToFP, 16);
+ EXPECT_EQ(16u, SIToFP.getBitWidth());
+ EXPECT_TRUE(SIToFP.isFullSet());
+
+ ConstantRange SIToFP2 = A.castOp(Instruction::SIToFP, 64);
+ ConstantRange C(APInt(64, -32768), APInt(64, 32768));
+ EXPECT_EQ(64u, SIToFP2.getBitWidth());
+ EXPECT_EQ(C, SIToFP2);
}
TEST_F(ConstantRangeTest, binaryAnd) {
More information about the llvm-commits
mailing list