Thank you all for the comments. Patch is updated. Bootstrap and no make check regression on X86-64. No make check regression with Cortex-M0 qemu. No performance changes for coremark, dhrystone, spec2000 and spec2006 on X86-64 and Cortex-A15.
For CSiBE, ARM Cortex-M0 result is a little better. A little regression for MIPS (less than 0.01%). diff --git a/gcc/ifcvt.c b/gcc/ifcvt.c index 653653f..7bb2578 100644 --- a/gcc/ifcvt.c +++ b/gcc/ifcvt.c @@ -1393,6 +1393,9 @@ noce_try_store_flag_mask (struct noce_if_info *if_info) if (target) { + int old_cost, new_cost, insn_cost; + int speed_p; + if (target != if_info->x) noce_emit_move_insn (if_info->x, target); @@ -1400,6 +1403,14 @@ noce_try_store_flag_mask (struct noce_if_info *if_info) if (!seq) return FALSE; + speed_p = optimize_bb_for_speed_p (BLOCK_FOR_INSN (if_info->insn_a)); + insn_cost = insn_rtx_cost (PATTERN (if_info->insn_a), speed_p); + old_cost = COSTS_N_INSNS (if_info->branch_cost) + insn_cost; + new_cost = seq_cost (seq, speed_p); + + if (new_cost > old_cost) + return FALSE; + emit_insn_before_setloc (seq, if_info->jump, INSN_LOCATION (if_info->insn_a)); return TRUE; diff --git a/gcc/testsuite/gcc.target/arm/ifcvt-size-check.c b/gcc/testsuite/gcc.target/arm/ifcvt-size-check.c new file mode 100644 index 0000000..43fa16b --- /dev/null +++ b/gcc/testsuite/gcc.target/arm/ifcvt-size-check.c @@ -0,0 +1,13 @@ +/* { dg-do assemble } */ +/* { dg-options "-mthumb -Os " } */ +/* { dg-require-effective-target arm_thumb1_ok } */ + +int +test (unsigned char iov_len, int count, int i) { + unsigned char bytes = 0; + if ((unsigned char) ((char) 127 - bytes) < iov_len) + return 22; + return 0; +} +/* { dg-final { object-size text <= 12 } } */ > -----Original Message----- > From: Jeff Law [mailto:l...@redhat.com] > Sent: Thursday, October 30, 2014 1:27 PM > To: Zhenqiang Chen; gcc-patches@gcc.gnu.org > Subject: Re: [PATCH, ifcvt] Check size cost in noce_try_store_flag_mask > > On 10/26/14 19:53, Zhenqiang Chen wrote: > > Hi, > > > > Function noce_try_store_flag_mask converts "if (test) x = 0;" to "x &= > > -(test == 0);" > > > > But from code size view, "x &= -(test == 0);" might have more > > instructions than "if (test) x = 0;". The patch checks the cost to > > determine the conversion is valuable or not. > > > > Bootstrap and no make check regression on X86-64. > > No make check regression with Cortex-M0 qemu. > > For CSiBE, ARM Cortex-m0 result is a little better. A little > > regression for MIPS. Roughly no change for PowerPC. > > > > OK for trunk? > > > > Thanks! > > -Zhenqiang > > > > ChangeLog: > > 2014-10-27 Zhenqiang Chen <zhenqiang.c...@arm.com> > > > > * ifcvt.c (noce_try_store_flag_mask): Check rtx cost. > > > > testsuite/ChangeLog: > > 2014-10-27 Zhenqiang Chen <zhenqiang.c...@arm.com> > > > > * gcc.target/arm/ifcvt-size-check.c: New test. > > > > diff --git a/gcc/ifcvt.c b/gcc/ifcvt.c index 949d2b4..3abd518 100644 > > --- a/gcc/ifcvt.c > > +++ b/gcc/ifcvt.c > > @@ -1393,6 +1393,14 @@ noce_try_store_flag_mask (struct noce_if_info > > *if_info) > > if (!seq) > > return FALSE; > > > > + if (optimize_function_for_size_p (cfun)) > > + { > > + int old_cost = COSTS_N_INSNS (if_info->branch_cost + 1); > > + int new_cost = seq_cost (seq, 0); > > + if (new_cost > old_cost) > > + return FALSE; > > + } > > + > As Andrew pointed out, there's really no reason to limit this to cases where > we're optimizing for size. So that check should be removed. > > You should also engage with Michael to determine if the MIPS regressions > are significant enough to warrant deeper investigation. My gut tells me that > if MIPS is regressing because of this, then that's going to be an issue in the > MIPS costing model that the MIPS folks will ultimately need to fix. > > jeff >