<div dir="ltr">+ Duncan<br><div class="gmail_extra"><br><div class="gmail_quote">On Sat, Jul 12, 2014 at 1:54 AM, suyog <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">Hi Duncan,<br>
<br>
Thanks for your review.<br>
Couldn't spot the miscompiles earlier because i didn't comment out<br>
the 'simplify' call. Now i modified the code and tested almost every<br>
combination.<br>
<br>
I have modified the code as per your suggestions :<br>
- Made comments short and crisp as per your suggestion.<br>
- included lambda helper functions to return appropriate constant<br>
  and icmp instruction.<br>
- included logic for both constants -1 for ashr as suggested<br>
  (In my opinion if both constants are equal and not -1,<br>
   then final icmp would be icmp eq/ne A, 0. The Predicate won't<br>
   get inversed and will remain same. You had suggested using lambda<br>
   function here to get inversed predicate. Correct me if my analysis is wrong.)<br>
- used 'bool IsAShr' to avoid recalculation.<br>
- removed LShrOperator block which was causing miscompiles.<br>
- removed unnecessary shl/lshr distinction for exact and non exact<br>
- moved all test cases to new file icmp-ashr.ll<br>
- used msb_high/low instead of +ve/-ve and opposite msb<br>
- tried including all combinations of ne/eq + exact/non-exact + lshr/ashr<br>
<div class=""><br>
Please help in reviewing the patch.<br>
<br>
</div>Your comments/suggestions are valuable and most awaited. :)<br>
<br>
Thanks,<br>
<div class="">Suyog<br>
<br>
<a href="http://reviews.llvm.org/D4068" target="_blank">http://reviews.llvm.org/D4068</a><br>
<br>
Files:<br>
  lib/Transforms/InstCombine/InstCombineCompares.cpp<br>
</div>  test/Transforms/InstCombine/icmp-shr.ll<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></blockquote></div><br><br clear="all"><br>-- <br>With regards,<br>Suyog Sarda<br>
</div></div>