<div dir="ltr"><div><div>Thanks Rafael.<br><br></div>I observed the test cases modified. Will take care of fine tuning test cases in future. :)<br><br></div>Thanks a lot.<br></div><div class="gmail_extra"><br><br><div class="gmail_quote">
On Wed, Jun 4, 2014 at 9:19 PM, Rafael Espíndola <span dir="ltr"><<a href="mailto:rafael.espindola@gmail.com" target="_blank">rafael.espindola@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
r210186. I just changed a few things before commit:<br>
<br>
* checkRippleForAdd was using only two arguments.<br>
* It could use an early return.<br>
* Reduce the tests to use the fact that nsw is now set for any add<br>
that passes WillNotOverflowSignedAdd, so we don't need any sign<br>
extensions to complicate the test.<br>
<div><div class="h5"><br>
<br>
On 4 June 2014 02:21, Suyog Kamal Sarda <<a href="mailto:suyog.sarda@samsung.com">suyog.sarda@samsung.com</a>> wrote:<br>
> Hi Rafael,<br>
><br>
> Attached updated patch with minor changes - starting letter small of helper function + passing function parameters by const reference.<br>
><br>
> To keep the helper function generic, i created a local copy of Op0KnownZero as we require to clear MSB bit.<br>
> Please see if this looks good to you.<br>
><br>
> Thanks,<br>
> Suyog Sarda<br>
><br>
> ------- Original Message -------<br>
> Sender : Rafael Espíndola<<a href="mailto:rafael.espindola@gmail.com">rafael.espindola@gmail.com</a>><br>
> Date : Jun 03, 2014 23:53 (GMT+09:00)<br>
> Title : Re: Re: Re: [llvm] r209746 - InstCombine: Improvement to check if signed addition overflows.<br>
><br>
> I updated my patch with the test cases. Can you please check if this is correct?<br>
> If it looks good to you, can you please commit it on my behalf?<br>
><br>
> My patch also handles the case, where  one of the operand has only 1 bit that is not known to<br>
><br>
> be zero, but it is also not know to be one.<br>
> ((~RHSKnownZero).countpopulation == 1) will be able to determine if only one of the bit is unkown (it may be 1 or 0).<br>
><br>
> As far as the test case :<br>
><br>
> define i32 @foo(i32 %x, i32 %y) {<br>
>   %v1 = and i32 %y, 1<br>
>   %v2 = add i32 %x, %v1<br>
>   ret i32 %v2<br>
> }<br>
><br>
> We know that only one of the bit is unknown of one of the operand, it may be 1 or 0.<br>
> But for the other operand, we do not know if it has 0 at any higher bit position. Hence we do not add 'nsw' in this case.<br>
><br>
> Added this test case as well + test case to handle sext  i1(bitwidth = 1).<br>
><br>
> Awaiting for your review.<br>
><br>
><br>
</div></div><div class="">> The check for a bitwidth of one seems redundant. The updated computation of Op1OnePosition handles that case too, no?<br>
><br>
><br>
</div><div class="">> It looks like the tests can be simplified a bit further. They are still using numbered instead of named instructions for example.<br>
><br>
><br>
</div><div class="">> Please make sure the positive tests are cases that currently fail, so they demonstrate the new optimization capabilities. For example, we already add a nsw to  @ripple_nsw because of the current logic for handling both operands having more than one sign bit.<br>

><br>
><br>
</div>> Cheers,<br>
> Rafael<br>
</blockquote></div><br><br clear="all"><br>-- <br>With regards,<br>Suyog Sarda<br>
</div>