This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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

Attachment: fix_cond_cmp_2.patch
Description: Binary data


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]