Hi Eli,<div><br></div><div>Please find the updated patch and simple unit test attached. I would have liked to add a more thorough unit test, but I believe there's a bug in the unit tests also. Could you please confirm the following...</div>
<div><br></div><div>Looking at TEST(APIntTest, i1) in unittests/ADT/APIntTest.cpp, I see an APInt defined as:</div><div><br></div><div>> const APInt two(1, 2);</div><div><br></div><div>If I'm not mistaken, that's one bit representing the unsigned value '2'. So, effectively, that APInt is a '0'. Unless this was done deliberately (which is why I'm asking), I believe the unit tests following this definition are not doing what they are supposed to do.</div>
<div><br></div><div>If you can confirm, I can take a whack at straightening things out.</div><div><br></div><div>Thanks again,</div><div>Cameron</div><div><br><div class="gmail_quote">On Tue, Oct 11, 2011 at 10:09 PM, Eli Friedman <span dir="ltr"><<a href="mailto:eli.friedman@gmail.com">eli.friedman@gmail.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;"><div><div></div><div class="h5">On Tue, Oct 11, 2011 at 6:06 PM, Cameron McInally<br>
<<a href="mailto:cameron.mcinally@nyu.edu">cameron.mcinally@nyu.edu</a>> wrote:<br>
> Hey guys,<br>
><br>
> Constant folding for the rotate operators does not work currently. Here's a<br>
> fix...<br>
><br>
> royale mcinally/llvm> svn diff<br>
><br>
> Index: lib/Support/APInt.cpp<br>
><br>
> ===================================================================<br>
><br>
> --- lib/Support/APInt.cpp (revision 141748)<br>
><br>
> +++ lib/Support/APInt.cpp (working copy)<br>
><br>
> @@ -1334,8 +1334,8 @@<br>
><br>
>    // Don't get too fancy, just use existing shift/or facilities<br>
><br>
>    APInt hi(*this);<br>
><br>
>    APInt lo(*this);<br>
><br>
> -  hi.shl(rotateAmt);<br>
><br>
> -  lo.lshr(BitWidth - rotateAmt);<br>
><br>
> +  hi = hi.shl(rotateAmt);<br>
><br>
> +  lo = lo.lshr(BitWidth - rotateAmt);<br>
><br>
>    return hi | lo;<br>
><br>
>  }<br>
><br>
><br>
><br>
> @@ -1349,8 +1349,8 @@<br>
><br>
>    // Don't get too fancy, just use existing shift/or facilities<br>
><br>
>    APInt hi(*this);<br>
><br>
>    APInt lo(*this);<br>
><br>
> -  lo.lshr(rotateAmt);<br>
><br>
> -  hi.shl(BitWidth - rotateAmt);<br>
><br>
> +  lo = lo.lshr(rotateAmt);<br>
><br>
> +  hi = hi.shl(BitWidth - rotateAmt);<br>
><br>
>    return hi | lo;<br>
><br>
>  }<br>
<br>
</div></div>Err, wow.  Nice catch.  Would you mind adding a test in<br>
unittests/ADT/APIntTest.cpp ?    (Also, we prefer patches to be<br>
attached rather than inlined.)<br>
<font color="#888888"><br>
-Eli<br>
</font></blockquote></div><br></div>