[llvm] r301110 - Revert "[APInt] Add ashrInPlace method and implement ashr using it. Also fix a bug in the shift by BitWidth handling."

Renato Golin via llvm-commits llvm-commits at lists.llvm.org
Sun Apr 23 05:02:08 PDT 2017


Author: rengolin
Date: Sun Apr 23 07:02:07 2017
New Revision: 301110

URL: http://llvm.org/viewvc/llvm-project?rev=301110&view=rev
Log:
Revert "[APInt] Add ashrInPlace method and implement ashr using it. Also fix a bug in the shift by BitWidth handling."

This reverts commit r301094, as it broke all ARM self-hosting bots.

PR32754.

Modified:
    llvm/trunk/include/llvm/ADT/APInt.h
    llvm/trunk/include/llvm/ADT/APSInt.h
    llvm/trunk/lib/Support/APInt.cpp
    llvm/trunk/unittests/ADT/APIntTest.cpp

Modified: llvm/trunk/include/llvm/ADT/APInt.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/ADT/APInt.h?rev=301110&r1=301109&r2=301110&view=diff
==============================================================================
--- llvm/trunk/include/llvm/ADT/APInt.h (original)
+++ llvm/trunk/include/llvm/ADT/APInt.h Sun Apr 23 07:02:07 2017
@@ -198,9 +198,6 @@ private:
   /// out-of-line slow case for lshr.
   void lshrSlowCase(unsigned ShiftAmt);
 
-  /// out-of-line slow case for ashr.
-  void ashrSlowCase(unsigned ShiftAmt);
-
   /// out-of-line slow case for operator=
   void AssignSlowCase(const APInt &RHS);
 
@@ -908,26 +905,7 @@ public:
   /// \brief Arithmetic right-shift function.
   ///
   /// Arithmetic right-shift this APInt by shiftAmt.
-  APInt ashr(unsigned ShiftAmt) const {
-    APInt R(*this);
-    R.ashrInPlace(ShiftAmt);
-    return R;
-  }
-
-  /// Arithmetic right-shift this APInt by ShiftAmt in place.
-  void ashrInPlace(unsigned ShiftAmt) {
-    assert(ShiftAmt <= BitWidth && "Invalid shift amount");
-    if (isSingleWord()) {
-      int64_t SExtVAL = SignExtend64(VAL, BitWidth);
-      if (ShiftAmt == BitWidth)
-        VAL = SExtVAL >> (APINT_BITS_PER_WORD - 1); // undefined
-      else
-        VAL = SExtVAL >> ShiftAmt;
-      clearUnusedBits();
-      return;
-    }
-    ashrSlowCase(ShiftAmt);
-  }
+  APInt ashr(unsigned shiftAmt) const;
 
   /// \brief Logical right-shift function.
   ///
@@ -969,14 +947,7 @@ public:
   /// \brief Arithmetic right-shift function.
   ///
   /// Arithmetic right-shift this APInt by shiftAmt.
-  APInt ashr(const APInt &ShiftAmt) const {
-    APInt R(*this);
-    R.ashrInPlace(ShiftAmt);
-    return R;
-  }
-
-  /// Arithmetic right-shift this APInt by shiftAmt in place.
-  void ashrInPlace(const APInt &shiftAmt);
+  APInt ashr(const APInt &shiftAmt) const;
 
   /// \brief Logical right-shift function.
   ///

Modified: llvm/trunk/include/llvm/ADT/APSInt.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/ADT/APSInt.h?rev=301110&r1=301109&r2=301110&view=diff
==============================================================================
--- llvm/trunk/include/llvm/ADT/APSInt.h (original)
+++ llvm/trunk/include/llvm/ADT/APSInt.h Sun Apr 23 07:02:07 2017
@@ -128,7 +128,7 @@ public:
     if (IsUnsigned)
       lshrInPlace(Amt);
     else
-      ashrInPlace(Amt);
+      *this = *this >> Amt;
     return *this;
   }
 

Modified: llvm/trunk/lib/Support/APInt.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Support/APInt.cpp?rev=301110&r1=301109&r2=301110&view=diff
==============================================================================
--- llvm/trunk/lib/Support/APInt.cpp (original)
+++ llvm/trunk/lib/Support/APInt.cpp Sun Apr 23 07:02:07 2017
@@ -1029,42 +1029,89 @@ APInt APInt::sextOrSelf(unsigned width)
 
 /// Arithmetic right-shift this APInt by shiftAmt.
 /// @brief Arithmetic right-shift function.
-void APInt::ashrInPlace(const APInt &shiftAmt) {
-  ashrInPlace((unsigned)shiftAmt.getLimitedValue(BitWidth));
+APInt APInt::ashr(const APInt &shiftAmt) const {
+  return ashr((unsigned)shiftAmt.getLimitedValue(BitWidth));
 }
 
 /// Arithmetic right-shift this APInt by shiftAmt.
 /// @brief Arithmetic right-shift function.
-void APInt::ashrSlowCase(unsigned ShiftAmt) {
-  // Don't bother performing a no-op shift.
-  if (!ShiftAmt)
-    return;
-
-  bool Negative = isNegative();
-
-  // WordShift is the inter-part shift; BitShift is is intra-part shift.
-  unsigned Words = getNumWords();
-  unsigned WordShift = std::min(ShiftAmt / APINT_BITS_PER_WORD, Words);
-  unsigned BitShift = ShiftAmt % APINT_BITS_PER_WORD;
-
-  unsigned WordsToMove = Words - WordShift;
-  // Fastpath for moving by whole words.
-  if (BitShift == 0) {
-    std::memmove(pVal, pVal + WordShift, WordsToMove * APINT_WORD_SIZE);
+APInt APInt::ashr(unsigned shiftAmt) const {
+  assert(shiftAmt <= BitWidth && "Invalid shift amount");
+  // Handle a degenerate case
+  if (shiftAmt == 0)
+    return *this;
+
+  // Handle single word shifts with built-in ashr
+  if (isSingleWord()) {
+    if (shiftAmt == BitWidth)
+      return APInt(BitWidth, 0); // undefined
+    return APInt(BitWidth, SignExtend64(VAL, BitWidth) >> shiftAmt);
+  }
+
+  // If all the bits were shifted out, the result is, technically, undefined.
+  // We return -1 if it was negative, 0 otherwise. We check this early to avoid
+  // issues in the algorithm below.
+  if (shiftAmt == BitWidth) {
+    if (isNegative())
+      return APInt(BitWidth, WORD_MAX, true);
+    else
+      return APInt(BitWidth, 0);
+  }
+
+  // Create some space for the result.
+  uint64_t * val = new uint64_t[getNumWords()];
+
+  // Compute some values needed by the following shift algorithms
+  unsigned wordShift = shiftAmt % APINT_BITS_PER_WORD; // bits to shift per word
+  unsigned offset = shiftAmt / APINT_BITS_PER_WORD; // word offset for shift
+  unsigned breakWord = getNumWords() - 1 - offset; // last word affected
+  unsigned bitsInWord = whichBit(BitWidth); // how many bits in last word?
+  if (bitsInWord == 0)
+    bitsInWord = APINT_BITS_PER_WORD;
+
+  // If we are shifting whole words, just move whole words
+  if (wordShift == 0) {
+    // Move the words containing significant bits
+    for (unsigned i = 0; i <= breakWord; ++i)
+      val[i] = pVal[i+offset]; // move whole word
+
+    // Adjust the top significant word for sign bit fill, if negative
+    if (isNegative())
+      if (bitsInWord < APINT_BITS_PER_WORD)
+        val[breakWord] |= WORD_MAX << bitsInWord; // set high bits
   } else {
-    for (unsigned i = 0; i != WordsToMove; ++i) {
-      pVal[i] = pVal[i + WordShift] >> BitShift;
-      if (i + 1 != WordsToMove)
-        pVal[i] |= pVal[i + WordShift + 1] << (APINT_BITS_PER_WORD - BitShift);
-      else if (Negative)
-        pVal[i] |= WORD_MAX << (APINT_BITS_PER_WORD - BitShift);
+    // Shift the low order words
+    for (unsigned i = 0; i < breakWord; ++i) {
+      // This combines the shifted corresponding word with the low bits from
+      // the next word (shifted into this word's high bits).
+      val[i] = (pVal[i+offset] >> wordShift) |
+               (pVal[i+offset+1] << (APINT_BITS_PER_WORD - wordShift));
+    }
+
+    // Shift the break word. In this case there are no bits from the next word
+    // to include in this word.
+    val[breakWord] = pVal[breakWord+offset] >> wordShift;
+
+    // Deal with sign extension in the break word, and possibly the word before
+    // it.
+    if (isNegative()) {
+      if (wordShift > bitsInWord) {
+        if (breakWord > 0)
+          val[breakWord-1] |=
+            WORD_MAX << (APINT_BITS_PER_WORD - (wordShift - bitsInWord));
+        val[breakWord] |= WORD_MAX;
+      } else
+        val[breakWord] |= WORD_MAX << (bitsInWord - wordShift);
     }
   }
 
-  // Fill in the remainder based on the original sign.
-  std::memset(pVal + WordsToMove, Negative ? -1 : 0,
-              WordShift * APINT_WORD_SIZE);
-  clearUnusedBits();
+  // Remaining words are 0 or -1, just assign them.
+  uint64_t fillValue = (isNegative() ? WORD_MAX : 0);
+  for (unsigned i = breakWord+1; i < getNumWords(); ++i)
+    val[i] = fillValue;
+  APInt Result(val, BitWidth);
+  Result.clearUnusedBits();
+  return Result;
 }
 
 /// Logical right-shift this APInt by shiftAmt.

Modified: llvm/trunk/unittests/ADT/APIntTest.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/ADT/APIntTest.cpp?rev=301110&r1=301109&r2=301110&view=diff
==============================================================================
--- llvm/trunk/unittests/ADT/APIntTest.cpp (original)
+++ llvm/trunk/unittests/ADT/APIntTest.cpp Sun Apr 23 07:02:07 2017
@@ -288,7 +288,7 @@ TEST(APIntTest, i1) {
   EXPECT_EQ(zero, one.shl(1));
   EXPECT_EQ(one, one.shl(0));
   EXPECT_EQ(zero, one.lshr(1));
-  EXPECT_EQ(one, one.ashr(1));
+  EXPECT_EQ(zero, one.ashr(1));
 
   // Rotates.
   EXPECT_EQ(one, one.rotl(0));
@@ -2024,24 +2024,6 @@ TEST(APIntTest, LogicalRightShift) {
   EXPECT_EQ(0, neg_one.lshr(128));
 }
 
-TEST(APIntTest, ArithmeticRightShift) {
-  // Ensure we handle large shifts of multi-word.
-  const APInt signmin32(APInt::getSignedMinValue(32));
-  EXPECT_TRUE(signmin32.ashr(32).isAllOnesValue());
-
-  // Ensure we handle large shifts of multi-word.
-  const APInt umax32(APInt::getSignedMaxValue(32));
-  EXPECT_EQ(0, umax32.ashr(32));
-
-  // Ensure we handle large shifts of multi-word.
-  const APInt signmin128(APInt::getSignedMinValue(128));
-  EXPECT_TRUE(signmin128.ashr(128).isAllOnesValue());
-
-  // Ensure we handle large shifts of multi-word.
-  const APInt umax128(APInt::getSignedMaxValue(128));
-  EXPECT_EQ(0, umax128.ashr(128));
-}
-
 TEST(APIntTest, LeftShift) {
   APInt i256(APInt::getLowBitsSet(256, 2));
 




More information about the llvm-commits mailing list