[llvm] r219487 - APInt: Unfold return expressions so RVO can work.

[David Blaikie]

(sorry Ben, forgot to reply-all - here's the on-list reply)

On Fri, Oct 10, 2014 at 9:33 AM, Benjamin Kramer <benny.kra at gmail.com>
wrote:

>
> On 10.10.2014, at 18:26, David Blaikie <dblaikie at gmail.com> wrote:
>
> >
> >
> > On Fri, Oct 10, 2014 at 3:18 AM, Benjamin Kramer <
> benny.kra at googlemail.com> wrote:
> > Author: d0k
> > Date: Fri Oct 10 05:18:12 2014
> > New Revision: 219487
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=219487&view=rev
> > Log:
> > APInt: Unfold return expressions so RVO can work.
> >
> > Saves a couple of expensive deep copies. NFC.
> >
> > Perhaps somewhat annoying, but could we fix this more generically by
> providing an rvalue overload of clearUnusedBits? (that returns by value,
> moving out of the current object) I guess that'd still incur some moves,
> rather than the completely free NRVO.
>
> That works, but only for the cases where we return a temporary. There were
> cases with local variables, too.


Oh. Again, could add some std::move in to get sufficiently right behavior
there... but yeah, not necessarily better than what you've done.


> Anyways, I dropped that idea primarily because compiler support for
> r-value overloads is still bad (gcc 4.8.1+, msvc 2013 "nov ctp").
>

Hmm - pity. Though I believe we're primarily concerned about the
performance of LLVM when compiled with Clang/LLVM - we care about
correctness/buildability on other compilers, but assume anyone who cares
about performance will build with a native compiler, then bootstrap. LLVM's
performance is pretty tuned for LLVM's optimizers at this point, as I
understand it.

- Ben
>
> >
> >
> > Modified:
> >     llvm/trunk/lib/Support/APInt.cpp
> >
> > Modified: llvm/trunk/lib/Support/APInt.cpp
> > URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Support/APInt.cpp?rev=219487&r1=219486&r2=219487&view=diff
> >
> ==============================================================================
> > --- llvm/trunk/lib/Support/APInt.cpp (original)
> > +++ llvm/trunk/lib/Support/APInt.cpp Fri Oct 10 05:18:12 2014
> > @@ -454,8 +454,10 @@ APInt APInt::XorSlowCase(const APInt& RH
> >    for (unsigned i = 0; i < numWords; ++i)
> >      val[i] = pVal[i] ^ RHS.pVal[i];
> >
> > +  APInt Result(val, getBitWidth());
> >    // 0^0==1 so clear the high bits in case they got set.
> > -  return APInt(val, getBitWidth()).clearUnusedBits();
> > +  Result.clearUnusedBits();
> > +  return Result;
> >  }
> >
> >  APInt APInt::operator*(const APInt& RHS) const {
> > @@ -473,7 +475,8 @@ APInt APInt::operator+(const APInt& RHS)
> >      return APInt(BitWidth, VAL + RHS.VAL);
> >    APInt Result(BitWidth, 0);
> >    add(Result.pVal, this->pVal, RHS.pVal, getNumWords());
> > -  return Result.clearUnusedBits();
> > +  Result.clearUnusedBits();
> > +  return Result;
> >  }
> >
> >  APInt APInt::operator-(const APInt& RHS) const {
> > @@ -482,7 +485,8 @@ APInt APInt::operator-(const APInt& RHS)
> >      return APInt(BitWidth, VAL - RHS.VAL);
> >    APInt Result(BitWidth, 0);
> >    sub(Result.pVal, this->pVal, RHS.pVal, getNumWords());
> > -  return Result.clearUnusedBits();
> > +  Result.clearUnusedBits();
> > +  return Result;
> >  }
> >
> >  bool APInt::EqualSlowCase(const APInt& RHS) const {
> > @@ -1114,7 +1118,9 @@ APInt APInt::ashr(unsigned shiftAmt) con
> >    uint64_t fillValue = (isNegative() ? -1ULL : 0);
> >    for (unsigned i = breakWord+1; i < getNumWords(); ++i)
> >      val[i] = fillValue;
> > -  return APInt(val, BitWidth).clearUnusedBits();
> > +  APInt Result(val, BitWidth);
> > +  Result.clearUnusedBits();
> > +  return Result;
> >  }
> >
> >  /// Logical right-shift this APInt by shiftAmt.
> > @@ -1151,7 +1157,9 @@ APInt APInt::lshr(unsigned shiftAmt) con
> >    // If we are shifting less than a word, compute the shift with a
> simple carry
> >    if (shiftAmt < APINT_BITS_PER_WORD) {
> >      lshrNear(val, pVal, getNumWords(), shiftAmt);
> > -    return APInt(val, BitWidth).clearUnusedBits();
> > +    APInt Result(val, BitWidth);
> > +    Result.clearUnusedBits();
> > +    return Result;
> >    }
> >
> >    // Compute some values needed by the remaining shift algorithms
> > @@ -1164,7 +1172,9 @@ APInt APInt::lshr(unsigned shiftAmt) con
> >        val[i] = pVal[i+offset];
> >      for (unsigned i = getNumWords()-offset; i < getNumWords(); i++)
> >        val[i] = 0;
> > -    return APInt(val,BitWidth).clearUnusedBits();
> > +    APInt Result(val, BitWidth);
> > +    Result.clearUnusedBits();
> > +    return Result;
> >    }
> >
> >    // Shift the low order words
> > @@ -1178,7 +1188,9 @@ APInt APInt::lshr(unsigned shiftAmt) con
> >    // Remaining words are 0
> >    for (unsigned i = breakWord+1; i < getNumWords(); ++i)
> >      val[i] = 0;
> > -  return APInt(val, BitWidth).clearUnusedBits();
> > +  APInt Result(val, BitWidth);
> > +  Result.clearUnusedBits();
> > +  return Result;
> >  }
> >
> >  /// Left-shift this APInt by shiftAmt.
> > @@ -1211,7 +1223,9 @@ APInt APInt::shlSlowCase(unsigned shiftA
> >        val[i] = pVal[i] << shiftAmt | carry;
> >        carry = pVal[i] >> (APINT_BITS_PER_WORD - shiftAmt);
> >      }
> > -    return APInt(val, BitWidth).clearUnusedBits();
> > +    APInt Result(val, BitWidth);
> > +    Result.clearUnusedBits();
> > +    return Result;
> >    }
> >
> >    // Compute some values needed by the remaining shift algorithms
> > @@ -1224,7 +1238,9 @@ APInt APInt::shlSlowCase(unsigned shiftA
> >        val[i] = 0;
> >      for (unsigned i = offset; i < getNumWords(); i++)
> >        val[i] = pVal[i-offset];
> > -    return APInt(val,BitWidth).clearUnusedBits();
> > +    APInt Result(val, BitWidth);
> > +    Result.clearUnusedBits();
> > +    return Result;
> >    }
> >
> >    // Copy whole words from this to Result.
> > @@ -1235,7 +1251,9 @@ APInt APInt::shlSlowCase(unsigned shiftA
> >    val[offset] = pVal[0] << wordShift;
> >    for (i = 0; i < offset; ++i)
> >      val[i] = 0;
> > -  return APInt(val, BitWidth).clearUnusedBits();
> > +  APInt Result(val, BitWidth);
> > +  Result.clearUnusedBits();
> > +  return Result;
> >  }
> >
> >  APInt APInt::rotl(const APInt &rotateAmt) const {
> >
> >
> > _______________________________________________
> > llvm-commits mailing list
> > llvm-commits at cs.uiuc.edu
> > http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
> >
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20141010/e2f2a1a1/attachment.html>