[llvm] MathExtras: template'ize alignToPowerOf2 (PR #97814)

[Ramkumar Ramachandra via llvm-commits]

https://github.com/artagnon updated https://github.com/llvm/llvm-project/pull/97814

>From 8ef407dded3941f1037dbc4ff26d4718ffcfea0c Mon Sep 17 00:00:00 2001
From: Ramkumar Ramachandra <ramkumar.ramachandra at codasip.com>
Date: Fri, 5 Jul 2024 12:29:30 +0100
Subject: [PATCH 1/3] MathExtras: template'ize alignToPowerOf2

Follow up on 5627794 (MathExtras: avoid unnecessarily widening types) to
change the overflow behavior of alignToPowerOf2 to only overflow if the
result is not representable in the return type. This allows us to
template'ize it, and avoid unnecessarily widening the types of
arguments.
---
 llvm/include/llvm/Support/MathExtras.h    | 16 ++++++++++++----
 llvm/unittests/Support/MathExtrasTest.cpp |  7 +++++--
 2 files changed, 17 insertions(+), 6 deletions(-)

diff --git a/llvm/include/llvm/Support/MathExtras.h b/llvm/include/llvm/Support/MathExtras.h
index f6b1fdb6aba9ef..81a9d16909cdc9 100644
--- a/llvm/include/llvm/Support/MathExtras.h
+++ b/llvm/include/llvm/Support/MathExtras.h
@@ -488,13 +488,21 @@ constexpr uint64_t alignTo(uint64_t Value, uint64_t Align) {
   return CeilDiv * Align;
 }
 
+/// Will overflow only if result is not representable in T.
+template <typename U, typename V, typename T = common_uint<U, V>>
+constexpr T alignToPowerOf2(U Value, V Align) {
+  assert(Align != 0 && (Align & (Align - 1)) == 0 &&
+         "Align must be a power of 2");
+  T CeilDiv = divideCeil(Value, Align);
+  return CeilDiv * Align;
+}
+
+/// Fallback when arguments aren't integral.
 constexpr uint64_t alignToPowerOf2(uint64_t Value, uint64_t Align) {
   assert(Align != 0 && (Align & (Align - 1)) == 0 &&
          "Align must be a power of 2");
-  // Replace unary minus to avoid compilation error on Windows:
-  // "unary minus operator applied to unsigned type, result still unsigned"
-  uint64_t NegAlign = (~Align) + 1;
-  return (Value + Align - 1) & NegAlign;
+  uint64_t CeilDiv = divideCeil(Value, Align);
+  return CeilDiv * Align;
 }
 
 /// If non-zero \p Skew is specified, the return value will be a minimal integer
diff --git a/llvm/unittests/Support/MathExtrasTest.cpp b/llvm/unittests/Support/MathExtrasTest.cpp
index a557b61db97520..c0c7c301f1590e 100644
--- a/llvm/unittests/Support/MathExtrasTest.cpp
+++ b/llvm/unittests/Support/MathExtrasTest.cpp
@@ -218,8 +218,11 @@ TEST(MathExtras, AlignToPowerOf2) {
   EXPECT_EQ(24u, alignToPowerOf2(17, 8));
   EXPECT_EQ(0u, alignToPowerOf2(~0LL, 8));
   EXPECT_EQ(240u, alignToPowerOf2(240, 16));
-  EXPECT_EQ(static_cast<uint64_t>(std::numeric_limits<uint32_t>::max()) + 1,
-            alignToPowerOf2(std::numeric_limits<uint32_t>::max(), 2));
+
+  // Overflow.
+  EXPECT_EQ(0u, alignToPowerOf2(static_cast<uint8_t>(200),
+                                static_cast<uint8_t>(128)));
+  EXPECT_EQ(0u, alignToPowerOf2(std::numeric_limits<uint32_t>::max(), 2));
 }
 
 TEST(MathExtras, AlignDown) {

>From e0d0516b3cd7ac226657caa49b68302ced77521d Mon Sep 17 00:00:00 2001
From: Ramkumar Ramachandra <ramkumar.ramachandra at codasip.com>
Date: Mon, 19 Aug 2024 14:08:28 +0100
Subject: [PATCH 2/3] MathExtras: change implementation; address review

---
 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 81a9d16909cdc9..5202accc32e505 100644
--- a/llvm/include/llvm/Support/MathExtras.h
+++ b/llvm/include/llvm/Support/MathExtras.h
@@ -493,16 +493,16 @@ template <typename U, typename V, typename T = common_uint<U, V>>
 constexpr T alignToPowerOf2(U Value, V Align) {
   assert(Align != 0 && (Align & (Align - 1)) == 0 &&
          "Align must be a power of 2");
-  T CeilDiv = divideCeil(Value, Align);
-  return CeilDiv * Align;
+  T NegAlign = static_cast<T>(0) - Align;
+  return (Value + (Align - 1)) & NegAlign;
 }
 
 /// Fallback when arguments aren't integral.
 constexpr uint64_t alignToPowerOf2(uint64_t Value, uint64_t Align) {
   assert(Align != 0 && (Align & (Align - 1)) == 0 &&
          "Align must be a power of 2");
-  uint64_t CeilDiv = divideCeil(Value, Align);
-  return CeilDiv * Align;
+  uint64_t NegAlign = static_cast<uint64_t>(0) - Align;
+  return (Value + (Align - 1)) & NegAlign;
 }
 
 /// If non-zero \p Skew is specified, the return value will be a minimal integer

>From 4628a603ab17ac1784f08f0672cab7f55fa61d9c Mon Sep 17 00:00:00 2001
From: Ramkumar Ramachandra <ramkumar.ramachandra at codasip.com>
Date: Mon, 19 Aug 2024 14:20:49 +0100
Subject: [PATCH 3/3] MathExtras: fix nit

---
 llvm/include/llvm/Support/MathExtras.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/llvm/include/llvm/Support/MathExtras.h b/llvm/include/llvm/Support/MathExtras.h
index 5202accc32e505..6061bff010d5fb 100644
--- a/llvm/include/llvm/Support/MathExtras.h
+++ b/llvm/include/llvm/Support/MathExtras.h
@@ -501,7 +501,7 @@ constexpr T alignToPowerOf2(U Value, V Align) {
 constexpr uint64_t alignToPowerOf2(uint64_t Value, uint64_t Align) {
   assert(Align != 0 && (Align & (Align - 1)) == 0 &&
          "Align must be a power of 2");
-  uint64_t NegAlign = static_cast<uint64_t>(0) - Align;
+  uint64_t NegAlign = 0 - Align;
   return (Value + (Align - 1)) & NegAlign;
 }