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

suyog suyog.sarda at samsung.com
Fri Jul 11 13:24:46 PDT 2014


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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: D4068.11326.patch
Type: text/x-patch
Size: 20191 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140711/86c89ac8/attachment.bin>


More information about the llvm-commits mailing list