[PATCH, ARM] Generate conditional compares in Thumb2 state

[Jiangning Liu]

PING...

BTW, in patch fix_cond_cmp_2.patch, the file mode of thumb2.md is carelessly
changed, so please check attached new patch file fix_cond_cmp_3.patch.

Thanks,
-Jiangning

> -----Original Message-----
> From: Jiangning Liu [mailto:jiangning.liu@arm.com]
> Sent: Monday, August 08, 2011 2:01 PM
> To: 'Ramana Radhakrishnan'
> Cc: gcc-patches@gcc.gnu.org
> Subject: RE: [PATCH, ARM] Generate conditional compares in Thumb2 state
> 
> In attached new patch, arm_arch_thumb2 is changed to TARGET_THUMB2. I
> tried that with my patch command line option -mcpu=armv7-a9 doesn't
> generate IT instruction any longer, unless option "-mthumb" is being
> added.
> 
> All of my tests assume command line option -mthumb, while cortex-M0,
> cortex-M3 cortex-M4 are covered by options -mcpu=cortex-m0, -
> march=armv7-m, and -march=armv7e-m respectively.
> 
> As for the extra problem exposed by this specific case, may we treat it
> as a separate fix to decouple it with this one, and I can give follow
> up later on? I think it is a general problem not only for the
> particular pattern it/op/it/op. But I'm not sure how far we can go to
> optimize this kind of problems introduced by IT block. For this
> specific case, I see "if conversion" already generates conditional move
> before combination pass. So basically the peephole rules may probably
> work for most of the general scenarios. My initial thought is go over
> the rules introducing IT block and try to figure out all of the
> combination that two of this kinds of rules can be in sequential order.
> Maybe we only need to handle the most common case like this one. Since
> I do see a bunch of rules have something to do with problem, I'd like
> to look into all of them to give a most reasonable solution in a
> separate fix.
> 
> Does it make sense?
> 
> Thanks,
> -Jiangning
> 
> > -----Original Message-----
> > From: Ramana Radhakrishnan [mailto:ramana.radhakrishnan@linaro.org]
> > Sent: Friday, August 05, 2011 9:20 AM
> > To: Jiangning Liu
> > Cc: gcc-patches@gcc.gnu.org
> > Subject: Re: [PATCH, ARM] Generate conditional compares in Thumb2
> > state
> >
> > On 3 August 2011 08:48, Jiangning Liu <jiangning.liu@arm.com> wrote:
> > > This patch is to generate more conditional compare instructions in
> > Thumb2
> > > state. Given an example like below,
> > >
> > > int f(int i, int j)
> > > {
> > >  if ( (i == '+') || (j == '-') ) {
> > >    return i;
> > >  } else {
> > >    return j;
> > >  }
> > > }
> > >
> > > Without the patch, compiler generates the following codes,
> > >
> > >        sub     r2, r0, #43
> > >        rsbs    r3, r2, #0
> > >        adc     r3, r3, r2
> > >        cmp     r1, #45
> > >        it      eq
> > >        orreq   r3, r3, #1
> > >        cmp     r3, #0
> > >        it      eq
> > >        moveq   r0, r1
> > >        bx      lr
> > >
> > > With the patch, compiler can generate conditional jump like below,
> > >
> > >        cmp     r0, #43
> > >        it      ne
> > >        cmpne   r1, #45
> > >        it      ne
> > >        movne   r0, r1
> > >        bx      lr
> >
> >
> > Nice improvement but there could be a single it block to handle both
> > and thus you could make this even better with
> >
> > cmp r0, #43
> > itt ne
> > cmpne r1 ,#45
> > movne r0, r1
> >
> > The way to do this would be to try and split this post-reload
> > unfortunately into the cmp instruction and the conditional compare
> > with the appropriate instruction length - Then the backend has a
> > chance of merging some of this into a single instruction.
> > Unfortunately that won't be very straight-forward but that's a
> > direction we probably ought to proceed with in this case.
> >
> > In a number of places:
> >
> > > +   if (arm_arch_thumb2)
> >
> > Ah instead of this please use if (TARGET_THUMB2) - arm_arch_thumb2 is
> > true based on the architecture levels and not necessarily if the user
> > wants to generate Thumb code. I don't want an unnecessary IT
> > instruction being emitted in the ASM block in ARM state for v7-a and
> > above.
> >
> > > Tested against arm-none-eabi target and no regression found.
> >
> > Presumably for ARM and Thumb2 state ?
> >
> >
> > cheers
> > Ramana
-------------- next part --------------
A non-text attachment was scrubbed...
Name: fix_cond_cmp_3.patch
Type: application/octet-stream
Size: 11707 bytes
Desc: not available
URL: <http://gcc.gnu.org/pipermail/gcc-patches/attachments/20110810/c80b969b/attachment.obj>