[llvm] MathExtras: rewrite some methods to never overflow (PR #95556)
Ramkumar Ramachandra via llvm-commits
llvm-commits at lists.llvm.org
Sat Jun 15 01:20: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/5] 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/5] 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;
}
>From 2a565268e0ded2accef84cd535c5cba48df37dd3 Mon Sep 17 00:00:00 2001
From: Ramkumar Ramachandra <ramkumar.ramachandra at codasip.com>
Date: Fri, 14 Jun 2024 17:49:12 +0100
Subject: [PATCH 3/5] MathExtras: address review
---
llvm/include/llvm/Support/MathExtras.h | 11 +++++++----
llvm/unittests/Support/MathExtrasTest.cpp | 11 +++++++++++
2 files changed, 18 insertions(+), 4 deletions(-)
diff --git a/llvm/include/llvm/Support/MathExtras.h b/llvm/include/llvm/Support/MathExtras.h
index 7ae0797bc8314..02b9f3604d9b5 100644
--- a/llvm/include/llvm/Support/MathExtras.h
+++ b/llvm/include/llvm/Support/MathExtras.h
@@ -453,10 +453,10 @@ 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;
+ int64_t Bias = Denominator >= 0 ? -1 : 1;
bool SameSign = (Numerator >= 0) == (Denominator >= 0);
return SameSign ? Numerator / Denominator
- : -((-Numerator - Bias) / Denominator) - 1;
+ : -((Bias - Numerator) / Denominator) - 1;
}
/// Returns the remainder of the Euclidean division of LHS by RHS. Result is
@@ -467,11 +467,14 @@ inline int64_t mod(int64_t Numerator, int64_t Denominator) {
return Mod < 0 ? Mod + Denominator : Mod;
}
-/// Returns (Numerator / Denominator) rounded by round-half-up. Guranteed to
+/// Returns (Numerator / Denominator) rounded by round-half-up. Guaranteed to
/// never overflow.
inline uint64_t divideNearest(uint64_t Numerator, uint64_t Denominator) {
+ uint64_t Mod = Numerator % Denominator;
+ uint64_t HalfDenomFloor = Denominator / 2;
return (Numerator / Denominator) +
- (Denominator == 1 ? 0 : Numerator % Denominator >= Denominator / 2);
+ (Mod > HalfDenomFloor ||
+ ((Mod == HalfDenomFloor) & ~(Denominator & 1)));
}
/// 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 ce620a443756d..bd09bab9be004 100644
--- a/llvm/unittests/Support/MathExtrasTest.cpp
+++ b/llvm/unittests/Support/MathExtrasTest.cpp
@@ -462,12 +462,17 @@ TEST(MathExtras, DivideNearest) {
EXPECT_EQ(divideNearest(5, 4), 1u);
EXPECT_EQ(divideNearest(6, 4), 2u);
EXPECT_EQ(divideNearest(3, 1), 3u);
+ EXPECT_EQ(divideNearest(3, 6), 1u);
+ EXPECT_EQ(divideNearest(3, 7), 0u);
EXPECT_EQ(divideNearest(std::numeric_limits<uint32_t>::max(), 2),
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());
+ EXPECT_EQ(divideNearest(std::numeric_limits<uint64_t>::max() - 1,
+ std::numeric_limits<uint64_t>::max()),
+ 1u);
}
TEST(MathExtras, DivideCeil) {
@@ -477,6 +482,8 @@ TEST(MathExtras, DivideCeil) {
EXPECT_EQ(divideCeil(5, 4), 2u);
EXPECT_EQ(divideCeil(6, 4), 2u);
EXPECT_EQ(divideCeil(3, 1), 3u);
+ EXPECT_EQ(divideCeil(3, 6), 1u);
+ EXPECT_EQ(divideCeil(3, 7), 1u);
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),
@@ -500,6 +507,8 @@ TEST(MathExtras, DivideCeil) {
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);
+ EXPECT_EQ(divideCeilSigned(std::numeric_limits<int64_t>::min(), 1),
+ std::numeric_limits<int64_t>::min());
}
TEST(MathExtras, DivideFloorSigned) {
@@ -519,6 +528,8 @@ TEST(MathExtras, DivideFloorSigned) {
std::numeric_limits<int32_t>::min() / 2);
EXPECT_EQ(divideFloorSigned(std::numeric_limits<int64_t>::max(), -2),
std::numeric_limits<int64_t>::min() / 2);
+ EXPECT_EQ(divideFloorSigned(std::numeric_limits<int64_t>::min(), 1),
+ std::numeric_limits<int64_t>::min());
}
TEST(MathExtras, Mod) {
>From ef03afc4d323b4fefa66f4a5d54710ff3080f3f9 Mon Sep 17 00:00:00 2001
From: Ramkumar Ramachandra <ramkumar.ramachandra at codasip.com>
Date: Fri, 14 Jun 2024 22:24:08 +0100
Subject: [PATCH 4/5] MathExtras: lift out TieBreaker
---
llvm/include/llvm/Support/MathExtras.h | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/llvm/include/llvm/Support/MathExtras.h b/llvm/include/llvm/Support/MathExtras.h
index 02b9f3604d9b5..65a17b6d82698 100644
--- a/llvm/include/llvm/Support/MathExtras.h
+++ b/llvm/include/llvm/Support/MathExtras.h
@@ -472,9 +472,8 @@ inline int64_t mod(int64_t Numerator, int64_t Denominator) {
inline uint64_t divideNearest(uint64_t Numerator, uint64_t Denominator) {
uint64_t Mod = Numerator % Denominator;
uint64_t HalfDenomFloor = Denominator / 2;
- return (Numerator / Denominator) +
- (Mod > HalfDenomFloor ||
- ((Mod == HalfDenomFloor) & ~(Denominator & 1)));
+ uint64_t TieBreaker = ((Mod == HalfDenomFloor) & ~(Denominator & 1));
+ return (Numerator / Denominator) + (Mod > HalfDenomFloor || TieBreaker);
}
/// Returns the largest uint64_t less than or equal to \p Value and is
>From 80db5549a34d0470b0e7679ba3d93fed232a882f Mon Sep 17 00:00:00 2001
From: Ramkumar Ramachandra <ramkumar.ramachandra at codasip.com>
Date: Sat, 15 Jun 2024 09:19:29 +0100
Subject: [PATCH 5/5] MathExtras: implemenet review suggestions
---
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 65a17b6d82698..5bcefe4b6c361 100644
--- a/llvm/include/llvm/Support/MathExtras.h
+++ b/llvm/include/llvm/Support/MathExtras.h
@@ -429,6 +429,7 @@ template <uint64_t Align> constexpr inline uint64_t alignTo(uint64_t Value) {
/// Returns the integer ceil(Numerator / Denominator). Unsigned version.
/// Guaranteed to never overflow.
inline uint64_t divideCeil(uint64_t Numerator, uint64_t Denominator) {
+ assert(Denominator && "Division by zero");
uint64_t Bias = (Numerator != 0);
return (Numerator - Bias) / Denominator + Bias;
}
@@ -456,7 +457,7 @@ inline int64_t divideFloorSigned(int64_t Numerator, int64_t Denominator) {
int64_t Bias = Denominator >= 0 ? -1 : 1;
bool SameSign = (Numerator >= 0) == (Denominator >= 0);
return SameSign ? Numerator / Denominator
- : -((Bias - Numerator) / Denominator) - 1;
+ : (Numerator - Bias) / Denominator - 1;
}
/// Returns the remainder of the Euclidean division of LHS by RHS. Result is
@@ -470,10 +471,9 @@ inline int64_t mod(int64_t Numerator, int64_t Denominator) {
/// Returns (Numerator / Denominator) rounded by round-half-up. Guaranteed to
/// never overflow.
inline uint64_t divideNearest(uint64_t Numerator, uint64_t Denominator) {
+ assert(Denominator && "Division by zero");
uint64_t Mod = Numerator % Denominator;
- uint64_t HalfDenomFloor = Denominator / 2;
- uint64_t TieBreaker = ((Mod == HalfDenomFloor) & ~(Denominator & 1));
- return (Numerator / Denominator) + (Mod > HalfDenomFloor || TieBreaker);
+ return (Numerator / Denominator) + (Mod > (Denominator - 1) / 2);
}
/// Returns the largest uint64_t less than or equal to \p Value and is
More information about the llvm-commits
mailing list