Re: [PATCH][ARM] Fix FAIL pr46975

[Richard Earnshaw <rearnsha at arm dot com>]

On 01/07/13 12:05, Kyrylo Tkachov wrote:
> Hi Richard,
>
> > > This hurts code size.
> > > Therefore I've disabled the new peephole2 for optimize_insn_for_size_p
> > so that
> > > the original peephole before r200197 is used when optimising for size.
> > >
> > > I've also added a test to confirm that the new peephole2 for the non-CC
> > > setting variants is being used when optimising for speed, since the
> > large code
> > > should be more efficient because the scheduler won't have to deal with
> > > condition-code setting.
> > >
> > > Ok for trunk?
> > >
> > > Thanks,
> > > Kyrill
> > >
> > > gcc/
> > > 2013-06-21  Kyrylo Tkachov  <kyrylo.tkachov@arm.com>
> > >
> > > 	* config/arm/arm.md (peepholes for eq (reg1) (reg2/imm)):
> > > 	Use the flag setting variants when optimising for size.
> > >
> > >
> > > gcc/testsuite/
> > > 2013-06-21  Kyrylo Tkachov  <kyrylo.tkachov@arm.com>
> > >
> > > 	* gcc.target/arm/pr46975-2.c: New test.
> >
> > Not ok.
> >
> > This re-introduces the bug that Meador was trying to fix.
> >
> > You need to ensure that the condition code register is dead before
> > applying this peephole.
>
> Thanks for the review.
>
> How about this? I'm reusing Meadors' peephole2 and guard the application by
> checking the CC reg for deadness with peep2_regno_dead_p.
>
> Kyrill
>
> gcc/
> 2013-07-01  Kyrylo Tkachov  <kyrylo.tkachov@arm.com>
>              Meador Inge  <meadori@codesourcery.com>
>
> 	* config/arm/arm.md (peepholes for eq (reg1) (reg2/imm)):
> 	Use the flag setting variants when optimising for size.
>
> 	
> gcc/testsuite/
> 2013-07-01  Kyrylo Tkachov  <kyrylo.tkachov@arm.com>
>              Meador Inge  <meadori@codesourcery.com>
>
> 	* gcc.target/arm/pr46975-2.c: New test.

+;; Rd = (eq (reg1) (reg2/imm))	// optimize for size on Thumb2
+;;	subs  T1, Reg1, reg2
+;;	negs Rd, T1
+;;	adcs  Rd, Rd, T1

Only the second operation has to be flag setting.  A later pass will 
convert the first and third instructions to flag clobbering if that's 
the right thing to do on the target (which it currently always is, but 
may not always remain so).


+/* { dg-options "-mthumb -O2" } */
+/* { dg-require-effective-target arm_thumb2_ok } */
+/* { dg-final { scan-assembler "sub" } } */
+/* { dg-final { scan-assembler "clz" } } */
+/* { dg-final { scan-assembler "lsrs" } } */

Similarly, this doesn't need to assume that the final instruction will 
clobber the flags: scanning for lsr is enough (scanning for lsr.*#5 
would be even better).

What about a test for arm state and armv5 or later?

R.