[llvm] MathExtras: rewrite two methods to never overflow (PR #95556)
Ramkumar Ramachandra via llvm-commits
llvm-commits at lists.llvm.org
Fri Jun 14 08:58:36 PDT 2024
https://github.com/artagnon updated https://github.com/llvm/llvm-project/pull/95556
>From 74144e16408be77f152eb98116b96f7550d13c3e Mon Sep 17 00:00:00 2001
From: Ramkumar Ramachandra <ramkumar.ramachandra at codasip.com>
Date: Fri, 14 Jun 2024 10:30:03 +0100
Subject: [PATCH 1/2] MathExtras: rewrite two methods to never overflow
Currently, divideCeil may overflow due to its dependency on alignTo, and
there is no way to avoid overflowing in alignTo. Remove this dependency,
and rewrite divideCeil to never overflow. Also rewrite divideNearest to
never overflow. Clarify divideCeilSigned and divideFloorSigned to
indicate that they can never overflow.
---
llvm/include/llvm/Support/MathExtras.h | 28 +++++++++++++++--------
llvm/unittests/Support/MathExtrasTest.cpp | 27 ++++++++++++++++++++--
2 files changed, 43 insertions(+), 12 deletions(-)
diff --git a/llvm/include/llvm/Support/MathExtras.h b/llvm/include/llvm/Support/MathExtras.h
index 05d87e176dec1..74815b7377ca5 100644
--- a/llvm/include/llvm/Support/MathExtras.h
+++ b/llvm/include/llvm/Support/MathExtras.h
@@ -385,6 +385,8 @@ inline uint64_t PowerOf2Ceil(uint64_t A) {
/// alignTo(~0LL, 8) = 0
/// alignTo(321, 255) = 510
/// \endcode
+///
+/// May overflow.
inline uint64_t alignTo(uint64_t Value, uint64_t Align) {
assert(Align != 0u && "Align can't be 0.");
return (Value + Align - 1) / Align * Align;
@@ -424,33 +426,37 @@ template <uint64_t Align> constexpr inline uint64_t alignTo(uint64_t Value) {
return (Value + Align - 1) / Align * Align;
}
-/// Returns the integer ceil(Numerator / Denominator). Unsigned integer version.
+/// Returns the integer ceil(Numerator / Denominator). Unsigned version.
+/// Guaranteed to never overflow.
inline uint64_t divideCeil(uint64_t Numerator, uint64_t Denominator) {
- return alignTo(Numerator, Denominator) / Denominator;
+ uint64_t Bias = (Numerator != 0);
+ return (Numerator - Bias) / Denominator + Bias;
}
-/// Returns the integer ceil(Numerator / Denominator). Signed integer version.
+/// Returns the integer ceil(Numerator / Denominator). Signed version.
+/// Guaranteed to never overflow.
inline int64_t divideCeilSigned(int64_t Numerator, int64_t Denominator) {
assert(Denominator && "Division by zero");
if (!Numerator)
return 0;
// C's integer division rounds towards 0.
- int64_t X = (Denominator > 0) ? -1 : 1;
+ int64_t Bias = (Denominator > 0 ? 1 : -1);
bool SameSign = (Numerator > 0) == (Denominator > 0);
- return SameSign ? ((Numerator + X) / Denominator) + 1
+ return SameSign ? (Numerator - Bias) / Denominator + 1
: Numerator / Denominator;
}
-/// Returns the integer floor(Numerator / Denominator). Signed integer version.
+/// Returns the integer floor(Numerator / Denominator). Signed version.
+/// Guaranteed to never overflow.
inline int64_t divideFloorSigned(int64_t Numerator, int64_t Denominator) {
assert(Denominator && "Division by zero");
if (!Numerator)
return 0;
// C's integer division rounds towards 0.
- int64_t X = (Denominator > 0) ? -1 : 1;
+ int64_t Bias = Denominator > 0 ? 1 : -1;
bool SameSign = (Numerator > 0) == (Denominator > 0);
return SameSign ? Numerator / Denominator
- : -((-Numerator + X) / Denominator) - 1;
+ : -((-Numerator - Bias) / Denominator) - 1;
}
/// Returns the remainder of the Euclidean division of LHS by RHS. Result is
@@ -461,9 +467,11 @@ inline int64_t mod(int64_t Numerator, int64_t Denominator) {
return Mod < 0 ? Mod + Denominator : Mod;
}
-/// Returns the integer nearest(Numerator / Denominator).
+/// Returns (Numerator / Denominator) rounded by round-half-up. Guranteed to
+/// never overflow.
inline uint64_t divideNearest(uint64_t Numerator, uint64_t Denominator) {
- return (Numerator + (Denominator / 2)) / Denominator;
+ return (Numerator / Denominator) +
+ (Denominator == 1 ? 0 : Numerator % Denominator >= Denominator / 2);
}
/// Returns the largest uint64_t less than or equal to \p Value and is
diff --git a/llvm/unittests/Support/MathExtrasTest.cpp b/llvm/unittests/Support/MathExtrasTest.cpp
index bcccb963c96ae..ce620a443756d 100644
--- a/llvm/unittests/Support/MathExtrasTest.cpp
+++ b/llvm/unittests/Support/MathExtrasTest.cpp
@@ -459,15 +459,30 @@ TEST(MathExtras, DivideNearest) {
EXPECT_EQ(divideNearest(14, 3), 5u);
EXPECT_EQ(divideNearest(15, 3), 5u);
EXPECT_EQ(divideNearest(0, 3), 0u);
+ EXPECT_EQ(divideNearest(5, 4), 1u);
+ EXPECT_EQ(divideNearest(6, 4), 2u);
+ EXPECT_EQ(divideNearest(3, 1), 3u);
EXPECT_EQ(divideNearest(std::numeric_limits<uint32_t>::max(), 2),
- 2147483648u);
+ std::numeric_limits<uint32_t>::max() / 2 + 1);
+ EXPECT_EQ(divideNearest(std::numeric_limits<uint64_t>::max(), 2),
+ std::numeric_limits<uint64_t>::max() / 2 + 1);
+ EXPECT_EQ(divideNearest(std::numeric_limits<uint64_t>::max(), 1),
+ std::numeric_limits<uint64_t>::max());
}
TEST(MathExtras, DivideCeil) {
EXPECT_EQ(divideCeil(14, 3), 5u);
EXPECT_EQ(divideCeil(15, 3), 5u);
EXPECT_EQ(divideCeil(0, 3), 0u);
- EXPECT_EQ(divideCeil(std::numeric_limits<uint32_t>::max(), 2), 2147483648u);
+ EXPECT_EQ(divideCeil(5, 4), 2u);
+ EXPECT_EQ(divideCeil(6, 4), 2u);
+ EXPECT_EQ(divideCeil(3, 1), 3u);
+ EXPECT_EQ(divideCeil(std::numeric_limits<uint32_t>::max(), 2),
+ std::numeric_limits<uint32_t>::max() / 2 + 1);
+ EXPECT_EQ(divideCeil(std::numeric_limits<uint64_t>::max(), 2),
+ std::numeric_limits<uint64_t>::max() / 2 + 1);
+ EXPECT_EQ(divideCeil(std::numeric_limits<uint64_t>::max(), 1),
+ std::numeric_limits<uint64_t>::max());
EXPECT_EQ(divideCeilSigned(14, 3), 5);
EXPECT_EQ(divideCeilSigned(15, 3), 5);
@@ -479,8 +494,12 @@ TEST(MathExtras, DivideCeil) {
EXPECT_EQ(divideCeilSigned(0, -3), 0);
EXPECT_EQ(divideCeilSigned(std::numeric_limits<int32_t>::max(), 2),
std::numeric_limits<int32_t>::max() / 2 + 1);
+ EXPECT_EQ(divideCeilSigned(std::numeric_limits<int64_t>::max(), 2),
+ std::numeric_limits<int64_t>::max() / 2 + 1);
EXPECT_EQ(divideCeilSigned(std::numeric_limits<int32_t>::max(), -2),
std::numeric_limits<int32_t>::min() / 2 + 1);
+ EXPECT_EQ(divideCeilSigned(std::numeric_limits<int64_t>::max(), -2),
+ std::numeric_limits<int64_t>::min() / 2 + 1);
}
TEST(MathExtras, DivideFloorSigned) {
@@ -494,8 +513,12 @@ TEST(MathExtras, DivideFloorSigned) {
EXPECT_EQ(divideFloorSigned(0, -3), 0);
EXPECT_EQ(divideFloorSigned(std::numeric_limits<int32_t>::max(), 2),
std::numeric_limits<int32_t>::max() / 2);
+ EXPECT_EQ(divideFloorSigned(std::numeric_limits<int64_t>::max(), 2),
+ std::numeric_limits<int64_t>::max() / 2);
EXPECT_EQ(divideFloorSigned(std::numeric_limits<int32_t>::max(), -2),
std::numeric_limits<int32_t>::min() / 2);
+ EXPECT_EQ(divideFloorSigned(std::numeric_limits<int64_t>::max(), -2),
+ std::numeric_limits<int64_t>::min() / 2);
}
TEST(MathExtras, Mod) {
>From a21838fce779ae7aad30da949ac3203ee8ae42bb Mon Sep 17 00:00:00 2001
From: Ramkumar Ramachandra <ramkumar.ramachandra at codasip.com>
Date: Fri, 14 Jun 2024 16:57:50 +0100
Subject: [PATCH 2/2] MathExtras: address nit
---
llvm/include/llvm/Support/MathExtras.h | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/llvm/include/llvm/Support/MathExtras.h b/llvm/include/llvm/Support/MathExtras.h
index 74815b7377ca5..7ae0797bc8314 100644
--- a/llvm/include/llvm/Support/MathExtras.h
+++ b/llvm/include/llvm/Support/MathExtras.h
@@ -440,8 +440,8 @@ inline int64_t divideCeilSigned(int64_t Numerator, int64_t Denominator) {
if (!Numerator)
return 0;
// C's integer division rounds towards 0.
- int64_t Bias = (Denominator > 0 ? 1 : -1);
- bool SameSign = (Numerator > 0) == (Denominator > 0);
+ int64_t Bias = (Denominator >= 0 ? 1 : -1);
+ bool SameSign = (Numerator >= 0) == (Denominator >= 0);
return SameSign ? (Numerator - Bias) / Denominator + 1
: Numerator / Denominator;
}
@@ -453,8 +453,8 @@ inline int64_t divideFloorSigned(int64_t Numerator, int64_t Denominator) {
if (!Numerator)
return 0;
// C's integer division rounds towards 0.
- int64_t Bias = Denominator > 0 ? 1 : -1;
- bool SameSign = (Numerator > 0) == (Denominator > 0);
+ int64_t Bias = Denominator >= 0 ? 1 : -1;
+ bool SameSign = (Numerator >= 0) == (Denominator >= 0);
return SameSign ? Numerator / Denominator
: -((-Numerator - Bias) / Denominator) - 1;
}
More information about the llvm-commits
mailing list