[libc-commits] [PATCH] D139566: [libc] Fix undefined behavior in UInt<>::shift_right.

[Tue Ly via Phabricator via libc-commits]

lntue created this revision.
lntue added reviewers: michaelrj, sivachandra.
Herald added subscribers: ecnelises, tschuett.
Herald added projects: libc-project, All.
lntue requested review of this revision.

Fix undefined behavior of left-shifting uint64_t by 64 in
`UInt<>::shift_right` implementation.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D139566

Files:
  libc/src/__support/UInt.h
  libc/test/src/__support/uint_test.cpp


Index: libc/test/src/__support/uint_test.cpp
===================================================================
--- libc/test/src/__support/uint_test.cpp
+++ libc/test/src/__support/uint_test.cpp
@@ -352,6 +352,17 @@
   LL_UInt128 result6({0, 0});
   EXPECT_EQ((val2 >> 128), result6);
   EXPECT_EQ((val2 >> 256), result6);
+
+  LL_UInt128 v1({0x1111222233334444, 0xaaaabbbbccccdddd});
+  LL_UInt128 r1({0xaaaabbbbccccdddd, 0});
+  EXPECT_EQ((v1 >> 64), r1);
+
+  LL_UInt192 v2({0x1111222233334444, 0x5555666677778888, 0xaaaabbbbccccdddd});
+  LL_UInt192 r2({0x5555666677778888, 0xaaaabbbbccccdddd, 0});
+  LL_UInt192 r3({0xaaaabbbbccccdddd, 0, 0});
+  EXPECT_EQ((v2 >> 64), r2);
+  EXPECT_EQ((v2 >> 128), r3);
+  EXPECT_EQ((r2 >> 64), r3);
 }
 
 TEST(LlvmLibcUIntClassTest, AndTests) {
@@ -502,11 +513,8 @@
   } while (0)
 
 TEST(LlvmLibcUIntClassTest, QuickMulHiTests) {
-  // TODO(lntue): Investigate / Analyze the error bounds for other rounding
-  // modes.  It the error bounds seems to be able to reach to WordCount instead
-  // of WordCount - 1 in the CI environment.
-  TEST_QUICK_MUL_HI(128, 2);
-  TEST_QUICK_MUL_HI(192, 3);
-  TEST_QUICK_MUL_HI(256, 4);
-  TEST_QUICK_MUL_HI(512, 8);
+  TEST_QUICK_MUL_HI(128, 1);
+  TEST_QUICK_MUL_HI(192, 2);
+  TEST_QUICK_MUL_HI(256, 3);
+  TEST_QUICK_MUL_HI(512, 7);
 }
Index: libc/src/__support/UInt.h
===================================================================
--- libc/src/__support/UInt.h
+++ libc/src/__support/UInt.h
@@ -386,18 +386,39 @@
   }
 
   constexpr void shift_right(size_t s) {
+#ifdef __SIZEOF_INT128__
+    if constexpr (Bits == 128) {
+      // Use builtin 128 bits if available;
+      if (s >= 128) {
+        val[0] = 0;
+        val[1] = 0;
+        return;
+      }
+      __uint128_t tmp = __uint128_t(val[0]) + (__uint128_t(val[1]) << 64);
+      tmp >>= s;
+      val[0] = uint64_t(tmp);
+      val[1] = uint64_t(tmp >> 64);
+      return;
+    }
+#endif // __SIZEOF_INT128__
+
     const size_t drop = s / 64;  // Number of words to drop
     const size_t shift = s % 64; // Bit shift in the remaining words.
 
     size_t i = 0;
 
     if (drop < WordCount) {
-      size_t j = drop;
-      for (; j < WordCount - 1; ++i, ++j) {
-        val[i] = (val[j] >> shift) | (val[j + 1] << (64 - shift));
+      if (shift > 0) {
+        for (size_t j = drop; j < WordCount - 1; ++i, ++j) {
+          val[i] = (val[j] >> shift) | (val[j + 1] << (64 - shift));
+        }
+        val[i] = val[WordCount - 1] >> shift;
+        ++i;
+      } else {
+        for (size_t j = drop; j < WordCount; ++i, ++j) {
+          val[i] = val[j];
+        }
       }
-      val[i] = val[j] >> shift;
-      ++i;
     }
 
     for (; i < WordCount; ++i) {


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D139566.480995.patch
Type: text/x-patch
Size: 2723 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/libc-commits/attachments/20221207/e6892f6a/attachment.bin>