<div dir="ltr">(sorry Ben, forgot to reply-all - here's the on-list reply)<br><div class="gmail_extra"><br><div class="gmail_quote">On Fri, Oct 10, 2014 at 9:33 AM, Benjamin Kramer <span dir="ltr"><<a href="mailto:benny.kra@gmail.com" target="_blank">benny.kra@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><span class=""><br>
On 10.10.2014, at 18:26, David Blaikie <<a href="mailto:dblaikie@gmail.com">dblaikie@gmail.com</a>> wrote:<br>
<br>
><br>
><br>
> On Fri, Oct 10, 2014 at 3:18 AM, Benjamin Kramer <<a href="mailto:benny.kra@googlemail.com">benny.kra@googlemail.com</a>> wrote:<br>
> Author: d0k<br>
> Date: Fri Oct 10 05:18:12 2014<br>
> New Revision: 219487<br>
><br>
> URL: <a href="http://llvm.org/viewvc/llvm-project?rev=219487&view=rev" target="_blank">http://llvm.org/viewvc/llvm-project?rev=219487&view=rev</a><br>
> Log:<br>
> APInt: Unfold return expressions so RVO can work.<br>
><br>
> Saves a couple of expensive deep copies. NFC.<br>
><br>
> 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.<br>
<br>
</span>That works, but only for the cases where we return a temporary. There were cases with local variables, too. </blockquote><div><div style="font-family:monospace"><br class="">Oh. Again, could add some std::move in to get sufficiently right behavior there... but yeah, not necessarily better than what you've done.</div><span class="im" style="font-family:monospace"><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">Anyways, I dropped that idea primarily because compiler support for r-value overloads is still bad (gcc 4.8.1+, msvc 2013 "nov ctp").<br></blockquote><div><br></div></span><div style="font-family:monospace">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.</div></div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
- Ben<br>
<div class=""><div class="h5"><br>
><br>
><br>
> Modified:<br>
> llvm/trunk/lib/Support/APInt.cpp<br>
><br>
> Modified: llvm/trunk/lib/Support/APInt.cpp<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Support/APInt.cpp?rev=219487&r1=219486&r2=219487&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Support/APInt.cpp?rev=219487&r1=219486&r2=219487&view=diff</a><br>
> ==============================================================================<br>
> --- llvm/trunk/lib/Support/APInt.cpp (original)<br>
> +++ llvm/trunk/lib/Support/APInt.cpp Fri Oct 10 05:18:12 2014<br>
> @@ -454,8 +454,10 @@ APInt APInt::XorSlowCase(const APInt& RH<br>
> for (unsigned i = 0; i < numWords; ++i)<br>
> val[i] = pVal[i] ^ RHS.pVal[i];<br>
><br>
> + APInt Result(val, getBitWidth());<br>
> // 0^0==1 so clear the high bits in case they got set.<br>
> - return APInt(val, getBitWidth()).clearUnusedBits();<br>
> + Result.clearUnusedBits();<br>
> + return Result;<br>
> }<br>
><br>
> APInt APInt::operator*(const APInt& RHS) const {<br>
> @@ -473,7 +475,8 @@ APInt APInt::operator+(const APInt& RHS)<br>
> return APInt(BitWidth, VAL + RHS.VAL);<br>
> APInt Result(BitWidth, 0);<br>
> add(Result.pVal, this->pVal, RHS.pVal, getNumWords());<br>
> - return Result.clearUnusedBits();<br>
> + Result.clearUnusedBits();<br>
> + return Result;<br>
> }<br>
><br>
> APInt APInt::operator-(const APInt& RHS) const {<br>
> @@ -482,7 +485,8 @@ APInt APInt::operator-(const APInt& RHS)<br>
> return APInt(BitWidth, VAL - RHS.VAL);<br>
> APInt Result(BitWidth, 0);<br>
> sub(Result.pVal, this->pVal, RHS.pVal, getNumWords());<br>
> - return Result.clearUnusedBits();<br>
> + Result.clearUnusedBits();<br>
> + return Result;<br>
> }<br>
><br>
> bool APInt::EqualSlowCase(const APInt& RHS) const {<br>
> @@ -1114,7 +1118,9 @@ APInt APInt::ashr(unsigned shiftAmt) con<br>
> uint64_t fillValue = (isNegative() ? -1ULL : 0);<br>
> for (unsigned i = breakWord+1; i < getNumWords(); ++i)<br>
> val[i] = fillValue;<br>
> - return APInt(val, BitWidth).clearUnusedBits();<br>
> + APInt Result(val, BitWidth);<br>
> + Result.clearUnusedBits();<br>
> + return Result;<br>
> }<br>
><br>
> /// Logical right-shift this APInt by shiftAmt.<br>
> @@ -1151,7 +1157,9 @@ APInt APInt::lshr(unsigned shiftAmt) con<br>
> // If we are shifting less than a word, compute the shift with a simple carry<br>
> if (shiftAmt < APINT_BITS_PER_WORD) {<br>
> lshrNear(val, pVal, getNumWords(), shiftAmt);<br>
> - return APInt(val, BitWidth).clearUnusedBits();<br>
> + APInt Result(val, BitWidth);<br>
> + Result.clearUnusedBits();<br>
> + return Result;<br>
> }<br>
><br>
> // Compute some values needed by the remaining shift algorithms<br>
> @@ -1164,7 +1172,9 @@ APInt APInt::lshr(unsigned shiftAmt) con<br>
> val[i] = pVal[i+offset];<br>
> for (unsigned i = getNumWords()-offset; i < getNumWords(); i++)<br>
> val[i] = 0;<br>
> - return APInt(val,BitWidth).clearUnusedBits();<br>
> + APInt Result(val, BitWidth);<br>
> + Result.clearUnusedBits();<br>
> + return Result;<br>
> }<br>
><br>
> // Shift the low order words<br>
> @@ -1178,7 +1188,9 @@ APInt APInt::lshr(unsigned shiftAmt) con<br>
> // Remaining words are 0<br>
> for (unsigned i = breakWord+1; i < getNumWords(); ++i)<br>
> val[i] = 0;<br>
> - return APInt(val, BitWidth).clearUnusedBits();<br>
> + APInt Result(val, BitWidth);<br>
> + Result.clearUnusedBits();<br>
> + return Result;<br>
> }<br>
><br>
> /// Left-shift this APInt by shiftAmt.<br>
> @@ -1211,7 +1223,9 @@ APInt APInt::shlSlowCase(unsigned shiftA<br>
> val[i] = pVal[i] << shiftAmt | carry;<br>
> carry = pVal[i] >> (APINT_BITS_PER_WORD - shiftAmt);<br>
> }<br>
> - return APInt(val, BitWidth).clearUnusedBits();<br>
> + APInt Result(val, BitWidth);<br>
> + Result.clearUnusedBits();<br>
> + return Result;<br>
> }<br>
><br>
> // Compute some values needed by the remaining shift algorithms<br>
> @@ -1224,7 +1238,9 @@ APInt APInt::shlSlowCase(unsigned shiftA<br>
> val[i] = 0;<br>
> for (unsigned i = offset; i < getNumWords(); i++)<br>
> val[i] = pVal[i-offset];<br>
> - return APInt(val,BitWidth).clearUnusedBits();<br>
> + APInt Result(val, BitWidth);<br>
> + Result.clearUnusedBits();<br>
> + return Result;<br>
> }<br>
><br>
> // Copy whole words from this to Result.<br>
> @@ -1235,7 +1251,9 @@ APInt APInt::shlSlowCase(unsigned shiftA<br>
> val[offset] = pVal[0] << wordShift;<br>
> for (i = 0; i < offset; ++i)<br>
> val[i] = 0;<br>
> - return APInt(val, BitWidth).clearUnusedBits();<br>
> + APInt Result(val, BitWidth);<br>
> + Result.clearUnusedBits();<br>
> + return Result;<br>
> }<br>
><br>
> APInt APInt::rotl(const APInt &rotateAmt) const {<br>
><br>
><br>
> _______________________________________________<br>
> llvm-commits mailing list<br>
> <a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br>
> <a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br>
><br>
<br>
</div></div></blockquote></div><br></div></div>