Message ID | 4DD220C1.7000405@codesourcery.com |
---|---|
State | New |
Headers | show |
Hi, > 2011-05-05 Tom de Vries <tom@codesourcery.com> > > PR target/45098 > * tree-ssa-loop-ivopts.c (computation_cost): Prevent cost of 0. this looks strange. Something like cost = seq_cost (seq, speed); if (MEM_P (rslt)) the current code; else cost += rtx_cost (rslt, SET, speed)); would make more sense to me (if I understand correctly what you are trying to achieve). Zdenek > Index: gcc/tree-ssa-loop-ivopts.c > =================================================================== > --- gcc/tree-ssa-loop-ivopts.c (revision 173380) > +++ gcc/tree-ssa-loop-ivopts.c (working copy) > @@ -2862,7 +2862,9 @@ computation_cost (tree expr, bool speed) > default_rtl_profile (); > node->frequency = real_frequency; > > - cost = seq_cost (seq, speed); > + cost = (seq != NULL_RTX > + ? seq_cost (seq, speed) > + : (unsigned)rtx_cost (rslt, SET, speed)); > if (MEM_P (rslt)) > cost += address_cost (XEXP (rslt, 0), TYPE_MODE (type), > TYPE_ADDR_SPACE (type), speed);
Hi Zdenek, thanks for the review. On 05/18/2011 09:26 AM, Zdenek Dvorak wrote: >> 2011-05-05 Tom de Vries <tom@codesourcery.com> >> >> PR target/45098 >> * tree-ssa-loop-ivopts.c (computation_cost): Prevent cost of 0. > > this looks strange. Something like > > cost = seq_cost (seq, speed); > if (MEM_P (rslt)) > the current code; > else > cost += rtx_cost (rslt, SET, speed)); > > would make more sense to me (if I understand correctly what you are > trying to achieve). > Sorry for not putting an explanation in the first place. I'm trying to achieve that the cost of forcing an int into a reg is not 0. Currently, if the expansion results in an rtx expression rather than an insn, seq is NULL and seq_cost returns 0, after which computation_cost returns 0: ... (gdb) call debug_generic_expr (expr) 2000 (gdb) call debug_rtx (rslt) (const_int 2000 [0x7d0]) (gdb) p seq $5 = (rtx) 0x0 ... Using an assert on an x86_64 build, I found this case where seq != NULL_RTX: ... gdb) call debug_generic_expr (expr) (int) ((unsigned int) ivtmp.11 * 8) (gdb) call debug_rtx (rslt) (reg:SI 62) (gdb) call debug_rtx (seq) (insn 4 0 0 (parallel [ (set (reg:SI 62) (ashift:SI (subreg:SI (reg:DI 59 [ ivtmp.11 ]) 0) (const_int 3 [0x3]))) (clobber (reg:CC 17 flags)) ]) -1 (nil)) ... we don't need to additionally count a regcopy of r62 on top of seq_cost in this case. How about: ... @@ -2866,6 +2878,8 @@ computation_cost (tree expr, bool speed) if (MEM_P (rslt)) cost += address_cost (XEXP (rslt, 0), TYPE_MODE (type), TYPE_ADDR_SPACE (type), speed); + else if (!REG_P (rslt)) + cost += (unsigned)rtx_cost (rslt, SET, speed); return cost; } ... ? Thanks - Tom
Hi Zdenek, On 05/18/2011 05:24 PM, Zdenek Dvorak wrote: > Hi, > >> How about: >> ... >> @@ -2866,6 +2878,8 @@ computation_cost (tree expr, bool speed) >> if (MEM_P (rslt)) >> cost += address_cost (XEXP (rslt, 0), TYPE_MODE (type), >> TYPE_ADDR_SPACE (type), speed); >> + else if (!REG_P (rslt)) >> + cost += (unsigned)rtx_cost (rslt, SET, speed); >> >> return cost; >> } >> ... >> ? > > this looks ok to me > thanks for the review. > (the cast to unsigned is not necessary, though?) You're right, it's not, that was only necessary to prevent a warning in the conditional expression originally proposed. Checked in without cast. Thanks, - Tom
2011-05-05 Tom de Vries <tom@codesourcery.com> PR target/45098 * tree-ssa-loop-ivopts.c (computation_cost): Prevent cost of 0. Index: gcc/tree-ssa-loop-ivopts.c =================================================================== --- gcc/tree-ssa-loop-ivopts.c (revision 173380) +++ gcc/tree-ssa-loop-ivopts.c (working copy) @@ -2862,7 +2862,9 @@ computation_cost (tree expr, bool speed) default_rtl_profile (); node->frequency = real_frequency; - cost = seq_cost (seq, speed); + cost = (seq != NULL_RTX + ? seq_cost (seq, speed) + : (unsigned)rtx_cost (rslt, SET, speed)); if (MEM_P (rslt)) cost += address_cost (XEXP (rslt, 0), TYPE_MODE (type), TYPE_ADDR_SPACE (type), speed);