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....@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.radhakrish...@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....@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
fix_cond_cmp_3.patch
Description: Binary data
