[PATCH] Add custom lowering for add/sub with overflow ARM

[Louis Gerbarg]

Thanks. You are correct that it ends up a bit cleaner and shorter pulling this if/else into the switch. New patch attached.

Louis
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Add-custom-lowering-for-add-sub-with-overflow-intrin.patch
Type: application/octet-stream
Size: 8610 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140508/c30a63a2/attachment.obj>
-------------- next part --------------

On May 8, 2014, at 2:14 AM, Renato Golin <renato.golin at linaro.org> wrote:

> Hi Louis,
> 
> Thanks for the patch, the results do look great!
> 
> Comments inline...
> 
> On 7 May 2014 23:13, Louis Gerbarg <lgg at apple.com> wrote:
>> 1) I only registered for i32s. I think there were probably some issues with
>> i64s and i32 is going to be the more common case on ARM.
> 
> I agree. Let's get the legal types in first.
> 
> 
>> 2) I updated all the tests to use regexps and track variables.
> 
> Thanks, that was much better.
> 
> 
>> 3) I spent some time trying to track through and get it to go to ADDS/SUBS,
>> but in the end that proved to be more work than I thought it would be due to
>> the differences between ARM and ARM64 in how the CPSR is handled, and the
>> ARM models CMPs directly (where as ARM64 converts them to SUBS), and the
>> fact that ARM backend only has working plumbing for CMP, not CMN. This patch
>> is substantially better behavior than we currently have, and I would rather
>> get it in now than wait trying to eek out that last bit of optimization.
> 
> Right.
> 
> Regarding the patch, I think you can simplify getARMXALUOOp()
> considerably by merging the switch with the if/else statement just
> below.
> 
> About the CMN workaround, I'd add a FIXME to it, too.
> 
> cheers,
> --renato