[llvm-commits] LLVM patch to optimize integer ABS idiom for ARM target

[Evan Cheng]

Hi Ana,

Thanks for working on this. It does look like an important peephole optimization. Unfortunately I think this patch has some problems.

1. The instruction selection pattern is looking for a very specific case. Does it still work if the source is not a function argument? For example, if it's the return value of a function call, or a result of a computation, I don't think it will work with this patch. 
2. MOVrs and RSBccri are not needed. You can expand into MOVr with the optional def set to CPSR and a RSBri with the predicate operand filled in.
3. We want to avoid pseudo instructions that expands into multiple instructions. As you have noticed, this messes up scheduling. Probably the right solution is to expand the instructions at pre-RA scheduling time.
4. It's a bad idea to have a single instruction that's used for both ARM and Thumb2 mode.

My suggestion:
1. ISel should match to an instruction (say ABS, t2ABSri) that's marked with usesCustomInserter = 1.
2. Add support to lower ABS to MOVr and RSBri with optional def and predicate operands filled in. Please do the same for the Thumb2 variant.

Bonuses:
1. Is this sufficient to generate the best code sequence? e.g.
        movs    r0, r0
        rsbmi   r0, r0, #0

This is the best if r0 is a function input. But how about?
        add       r0, r1
        movs    r0, r0
        rsbmi   r0, r0, #0

Shouldn't we copy propagate the movs?
        adds     r0, r1
        rsbmi   r0, r0, #0

One possibility is the scheduler custom expansion code look for the instruction that defines the source and tack the optional def on that instruction. Any other ideas?

2. How to make the instruction selection code match more cases? My suggestion is to add a new target independent opcode ABS. For targets where this node is legal, i.e. ARM, dag combine can form this instruction rather than the sra + sub sequence. This way, you can write a simple pattern to match the instruction instead of C++ selection code.

Evan

On Sep 15, 2011, at 5:32 PM, Ana Pazos wrote:

> Hello,
>  
> I worked on an LLVM patch to optimize integer ABS idiom for the ARM target and would like to submit it to your review.
>  
> I experimented with EEMBC benchmarks, in particular MPEG encoding, and noted integer ABS computation happens frequently. Significant speed up was achieved with the  optimized idiom for ARM (20% for MPEG encoding).
>  
> Patch details:
>  
> LLVM lowers SELECT_CC nodes that represent an integer ABS pattern into ASR/ADD/XOR instructions.
>  
> It is possible to create an optimized machine idiom for integer ABS on ARM formed by MOVs/RSBmi predicated instructions.
>  
> This patch modifies ARM-specific files to implement the above optimized machine idiom for integer ABS.
>  
> Generation of the optimized integer ABS idiom is turned on by default. To turn this feature off set -disable-arm-int-abs feature flag.
>  
> abspatch.diff
> Changes to ARM-specific files to implement optimized integer ABS idiom.
>  
> abstestpatch.diff
> ARM/iabs.ll and Thumb/iabs.ll tests check for the non-optimized integer ABS idiom (ASR/ADD/XOR).
> When applying abspatch.diff these tests fail. So patched the tests to set -disable-arm-int-abs flag to prevent the compiler from generating optimized integer ABS pattern and allow the non-optimized idiom to be checked.
>  
> iabsopt.ll
> Similar to ARM/iabs.ll and Thumb/iabs.ll tests except that it checks for the optimized integer ABS idiom and checks for all possible test conditions.
>  
> failures.txt
> Failure report and explanation from running llvm/test and projects/test-suite on ARM.
> I noted failures running llvm/test (svn version 139318) and projects/test-suite (svn revision 139319) on ARM. Are these failures expected?
>  
> Thank you,
> Ana.
>  
> <abspatch.diff><abstestpatch.diff><failures.txt><iabsopt.ll>_______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20110916/5f65df24/attachment.html>