[PATCH] PR19958 wrong code at -O1 and above on x86_64-linux-gnu (InstCombine)

[suyog sarda]

+ Duncan

On Sat, Jul 12, 2014 at 1:54 AM, suyog <suyog.sarda at samsung.com> wrote:

> Hi Duncan,
>
> Thanks for your review.
> Couldn't spot the miscompiles earlier because i didn't comment out
> the 'simplify' call. Now i modified the code and tested almost every
> combination.
>
> I have modified the code as per your suggestions :
> - Made comments short and crisp as per your suggestion.
> - included lambda helper functions to return appropriate constant
>   and icmp instruction.
> - included logic for both constants -1 for ashr as suggested
>   (In my opinion if both constants are equal and not -1,
>    then final icmp would be icmp eq/ne A, 0. The Predicate won't
>    get inversed and will remain same. You had suggested using lambda
>    function here to get inversed predicate. Correct me if my analysis is
> wrong.)
> - used 'bool IsAShr' to avoid recalculation.
> - removed LShrOperator block which was causing miscompiles.
> - removed unnecessary shl/lshr distinction for exact and non exact
> - moved all test cases to new file icmp-ashr.ll
> - used msb_high/low instead of +ve/-ve and opposite msb
> - tried including all combinations of ne/eq + exact/non-exact + lshr/ashr
>
> Please help in reviewing the patch.
>
> Your comments/suggestions are valuable and most awaited. :)
>
> Thanks,
> Suyog
>
> http://reviews.llvm.org/D4068
>
> Files:
>   lib/Transforms/InstCombine/InstCombineCompares.cpp
>   test/Transforms/InstCombine/icmp-shr.ll
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
>


-- 
With regards,
Suyog Sarda
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140712/c5a9eb65/attachment.html>