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] Fix FAIL pr46975


Hi all,
Resurrecting this thread...

> 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).

So I tried removing the flag setting from the peephole for the sub and adc
instructions, and nothing ends up converting them to flag-setting instructions
and we end up with:
sub
negs
adc
which is suboptimal for size. Is combine supposed to do decide whether to use
flag-setting variants?
 
Thanks,
Kyrill





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