[PATCH] Fix private constructor for ScaledNumber.

Diego Novillo dnovillo at google.com
Wed Apr 29 13:32:44 PDT 2015


Hi dexonsmith,

It was using uint64_t instead of DigitsT. This was preventing
instantiations of ScaledNumber with anything other than uint64_t types.

In implementing the tests, I ran into another issue. Operators >>= and
<<= did not have variants for accepting other ScaledNumber as the shift
argument. This is expected by the SCALED_NUMBER_BOP.

I'm not sure it makes sense to allow shifting a ScaledNumber by another
ScaledNumber, so I generated a new template for shifting ScaledNumbers
using a new macro SCALED_NUMBER_SHIFT.

http://reviews.llvm.org/D9350

Files:
  include/llvm/Support/ScaledNumber.h
  unittests/Support/ScaledNumberTest.cpp

Index: include/llvm/Support/ScaledNumber.h
===================================================================
--- include/llvm/Support/ScaledNumber.h
+++ include/llvm/Support/ScaledNumber.h
@@ -514,7 +514,7 @@
       : Digits(Digits), Scale(Scale) {}
 
 private:
-  ScaledNumber(const std::pair<uint64_t, int16_t> &X)
+  ScaledNumber(const std::pair<DigitsT, int16_t> &X)
       : Digits(X.first), Scale(X.second) {}
 
 public:
@@ -732,10 +732,18 @@
 SCALED_NUMBER_BOP(-, -= )
 SCALED_NUMBER_BOP(*, *= )
 SCALED_NUMBER_BOP(/, /= )
-SCALED_NUMBER_BOP(<<, <<= )
-SCALED_NUMBER_BOP(>>, >>= )
 #undef SCALED_NUMBER_BOP
 
+#define SCALED_NUMBER_SHIFT(op, base)                                          \
+  template <class DigitsT>                                                     \
+  ScaledNumber<DigitsT> operator op(const ScaledNumber<DigitsT> &L,            \
+                                    int16_t Shift) {                           \
+    return ScaledNumber<DigitsT>(L) base Shift;                                \
+  }
+SCALED_NUMBER_SHIFT(<<, <<= )
+SCALED_NUMBER_SHIFT(>>, >>= )
+#undef SCALED_NUMBER_SHIFT
+
 template <class DigitsT>
 raw_ostream &operator<<(raw_ostream &OS, const ScaledNumber<DigitsT> &X) {
   return X.print(OS, 10);
Index: unittests/Support/ScaledNumberTest.cpp
===================================================================
--- unittests/Support/ScaledNumberTest.cpp
+++ unittests/Support/ScaledNumberTest.cpp
@@ -155,7 +155,7 @@
   EXPECT_EQ(SP32(0xaaaaaaab, -33), getQuotient32(1, 3));
   EXPECT_EQ(SP32(0xd5555555, -31), getQuotient32(5, 3));
 
-  // 64-bit division is hard to test, since divide64 doesn't canonicalized its
+  // 64-bit division is hard to test, since divide64 doesn't canonicalize its
   // output.  However, this is the algorithm the implementation uses:
   //
   // - Shift divisor right.
@@ -532,4 +532,17 @@
   EXPECT_EQ(SP64(0, 0), getDifference64(1, -64, 1, -1));
 }
 
+TEST(ScaledNumberHelpersTest, arithmeticOperators) {
+  EXPECT_EQ(ScaledNumber<uint32_t>(1, 3) + ScaledNumber<uint32_t>(1, 1),
+            ScaledNumber<uint32_t>(10, 0));
+  EXPECT_EQ(ScaledNumber<uint32_t>(1, 3) - ScaledNumber<uint32_t>(1, 1),
+            ScaledNumber<uint32_t>(6, 0));
+  EXPECT_EQ(ScaledNumber<uint32_t>(1, 3) * ScaledNumber<uint32_t>(1, 1),
+            ScaledNumber<uint32_t>(2, 3));
+  EXPECT_EQ(ScaledNumber<uint32_t>(1, 3) / ScaledNumber<uint32_t>(1, 1),
+            ScaledNumber<uint32_t>(1, 2));
+  EXPECT_EQ(ScaledNumber<uint32_t>(1, 3) >> 1, ScaledNumber<uint32_t>(1, 2));
+  EXPECT_EQ(ScaledNumber<uint32_t>(1, 3) << 1, ScaledNumber<uint32_t>(2, 3));
+}
+
 } // end namespace

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/
-------------- next part --------------
A non-text attachment was scrubbed...
Name: D9350.24655.patch
Type: text/x-patch
Size: 2651 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150429/4f1a1900/attachment.bin>


More information about the llvm-commits mailing list