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

Renato Golin renato.golin at linaro.org
Thu May 8 02:14:50 PDT 2014


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



More information about the llvm-commits mailing list