<div dir="ltr">BTW, I was curious why the tests looked more complicated than needed. It turns out we were not just adding nsw based on WillNotOverflowSignedAdd. I changed that in r210029. It should allow you to simplify the tests further.</div>
<div class="gmail_extra"><br><br><div class="gmail_quote">On 2 June 2014 10:02, 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"><div dir="ltr"><div><br></div><div>Something wrong with the patch:</div><div><br></div><div><div>patch -p0 < ~/Downloads/AddOverFlow.patch </div>
<div>patching file lib/Transforms/InstCombine/InstCombineAddSub.cpp</div><div>
patch: **** malformed patch at line 84: Index: test/Transforms/InstCombine/AddOverFlow.ll</div></div><div><br></div><div><div>+  %1 = sext i16 %x to i32</div></div><div><br></div><div>Please run "opt -instnamer" (or manually name the instructions), that make the test a lot easier to read.</div>

<div><br></div><div><br></div><div><br></div></div><div class="HOEnZb"><div class="h5"><div class="gmail_extra"><br><br><div class="gmail_quote">On 2 June 2014 08:55, Suyog Kamal Sarda <span dir="ltr"><<a href="mailto:suyog.sarda@samsung.com" target="_blank">suyog.sarda@samsung.com</a>></span> wrote:<br>

<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">




<div>
<p>Hi Rafael,</p>
<p> </p>
<p>I found the bug. Basically, this was giving problem for operands having bitwidth 1.</p>
<p>It was always returning true for 'WillNotOverFlowSignedAdd' and hence the test case :</p><div>
<p><em></em> </p>
<p><em>define i32 @foo(i32 %x, i1 %y) {<br>%v1 = sext i1 %y to i32<br>%v2 = add i32 %v1, -1<br>ret i32 %v2<br>}<br></em></p>
</div><p>was getting reduced to </p><div>
<p><em>define i32 @foo(i32 %x, i1 %y) {<br>%addconv = xor i1 %y, true<br>%v2 = sext i1 %addconv to i32<br>ret i32 %v2<br></em><em>}</em></p>
<p><em></em> </p>
</div><p>Because -1 is represented as 1 in 1 bit representation, and it was always returning true for NotSignedOverFlow, </p>
<p>the carry bit was getting ignored and the add was getting converted to xor operations (since 'xor' = 'add neglecting carry bit' ) </p>
<p>causing error.</p>
<p> </p>
<p>I have handled this condition in the updated patch -> return true only if one of the operand has 0 bit for BitWidth 1.</p>
<p>Added the above test case as well.</p>
<p> </p>
<p>Attaching updated patch. Please help in reviewing it.</p>
<p> </p>
<p>Thanks !    </p><div>
<p><br>Regards,</p>
<p>Suyog Sarda</p>
<p> </p>
<p>------- <b>Original Message</b> -------</p>
<p><b>Sender</b> : Rafael Espíndola<<a href="mailto:rafael.espindola@gmail.com" target="_blank">rafael.espindola@gmail.com</a>></p>
</div><p><b>Date</b> : May 31, 2014 04:06 (GMT+09:00)</p><div>
<p><b>Title</b> : Re: [llvm] r209746 - InstCombine: Improvement to check if signed addition overflows.</p>
<p> </p></div>> Strangely though, removing the symmetric call gives good result of<div><div><br>> test-func.ll you had provided (I donno how but it works, even though the<br>> second call would be dead).<br>
<br>Well, we have to understand what is wrong with it.<br><br>The testcase reduces to just<br><br>define i32 @foo(i32 %x, i1 %y) {<br>  %v1 = sext i1 %y to i32<br>  %v2 = add i32 %v1, -1<br>  ret i32 %v2<br>}<br><br>which with the patch gets converted to<br>

<br>define i32 @foo(i32 %x, i1 %y) {<br>  %addconv = xor i1 %y, true<br>  %v2 = sext i1 %addconv to i32<br>  ret i32 %v2<br>}<br><br>Can you step over instcombine and see what invalid transformation is being done?<br><br>

Cheers,<br>Rafael<br>
<p> </p>
<p> </p>
<p> </p>
</div></div><table>
<tbody>
<tr>
<td>
<p><img border="0" src="cid:BEI0XT4NZ5JE@namo.co.kr" width="520"></p></td></tr></tbody></table></div><img border="0" width="0" height="0"></blockquote></div><br></div>
</div></div></blockquote></div><br></div>