Message ID | 1348519113.27229.34.camel@gnopaine |
---|---|
State | New |
Headers | show |
On Mon, 24 Sep 2012, William J. Schmidt wrote: > In cases where pointers and ints are cast back and forth, SLSR can be > tricked into introducing a multiply where one of the operands is of > pointer type. Don't do that! > > Verified that the reduced test case in the PR is fixed with a > cross-compile to sh4-unknown-linux-gnu with -Os, which is the only known > situation where the replacement looks profitable. (It appears multiply > costs are underestimated.) > > Bootstrapped and tested on powerpc64-unknown-linux-gnu with no new > regressions. Ok for trunk? Ok. Btw, a multiply by/of a pointer in GIMPLE is done by casting to an appropriate unsigned type, doing the multiply, and then casting back to the pointer type. Just in case it _is_ profitable to do the transform (the patch seems to try to avoid the situation only?) Thanks, Richard. > Thanks, > Bill > > > 2012-09-24 Bill Schmidt <wschmidt@linux.vnet.ibm.com> > > * gimple-ssa-strength-reduction.c (analyze_increments): Don't > introduce a multiplication with a pointer operand. > > > Index: gcc/gimple-ssa-strength-reduction.c > =================================================================== > --- gcc/gimple-ssa-strength-reduction.c (revision 191665) > +++ gcc/gimple-ssa-strength-reduction.c (working copy) > @@ -2028,6 +2028,15 @@ analyze_increments (slsr_cand_t first_dep, enum ma > > incr_vec[i].cost = COST_INFINITE; > > + /* If we need to add an initializer, make sure we don't introduce > + a multiply by a pointer type, which can happen in certain cast > + scenarios. */ > + else if (!incr_vec[i].initializer > + && TREE_CODE (first_dep->stride) != INTEGER_CST > + && POINTER_TYPE_P (TREE_TYPE (first_dep->stride))) > + > + incr_vec[i].cost = COST_INFINITE; > + > /* For any other increment, if this is a multiply candidate, we > must introduce a temporary T and initialize it with > T_0 = stride * increment. When optimizing for speed, walk the > > >
On Tue, 2012-09-25 at 09:14 +0200, Richard Guenther wrote: > On Mon, 24 Sep 2012, William J. Schmidt wrote: > > > In cases where pointers and ints are cast back and forth, SLSR can be > > tricked into introducing a multiply where one of the operands is of > > pointer type. Don't do that! > > > > Verified that the reduced test case in the PR is fixed with a > > cross-compile to sh4-unknown-linux-gnu with -Os, which is the only known > > situation where the replacement looks profitable. (It appears multiply > > costs are underestimated.) > > > > Bootstrapped and tested on powerpc64-unknown-linux-gnu with no new > > regressions. Ok for trunk? > > Ok. Btw, a multiply by/of a pointer in GIMPLE is done by casting > to an appropriate unsigned type, doing the multiply, and then > casting back to the pointer type. Just in case it _is_ profitable > to do the transform (the patch seems to try to avoid the situation > only?) Ok, that's good to know, thanks. There's a general to-do in that area to make the whole casting part better than it is right now, and that should be addressed when I can get back to GCC and work on some of these things. I'll add a comment to that effect. Appreciate the information! Thanks, Bill > > Thanks, > Richard. > > > Thanks, > > Bill > > > > > > 2012-09-24 Bill Schmidt <wschmidt@linux.vnet.ibm.com> > > > > * gimple-ssa-strength-reduction.c (analyze_increments): Don't > > introduce a multiplication with a pointer operand. > > > > > > Index: gcc/gimple-ssa-strength-reduction.c > > =================================================================== > > --- gcc/gimple-ssa-strength-reduction.c (revision 191665) > > +++ gcc/gimple-ssa-strength-reduction.c (working copy) > > @@ -2028,6 +2028,15 @@ analyze_increments (slsr_cand_t first_dep, enum ma > > > > incr_vec[i].cost = COST_INFINITE; > > > > + /* If we need to add an initializer, make sure we don't introduce > > + a multiply by a pointer type, which can happen in certain cast > > + scenarios. */ > > + else if (!incr_vec[i].initializer > > + && TREE_CODE (first_dep->stride) != INTEGER_CST > > + && POINTER_TYPE_P (TREE_TYPE (first_dep->stride))) > > + > > + incr_vec[i].cost = COST_INFINITE; > > + > > /* For any other increment, if this is a multiply candidate, we > > must introduce a temporary T and initialize it with > > T_0 = stride * increment. When optimizing for speed, walk the > > > > > > >
Index: gcc/gimple-ssa-strength-reduction.c =================================================================== --- gcc/gimple-ssa-strength-reduction.c (revision 191665) +++ gcc/gimple-ssa-strength-reduction.c (working copy) @@ -2028,6 +2028,15 @@ analyze_increments (slsr_cand_t first_dep, enum ma incr_vec[i].cost = COST_INFINITE; + /* If we need to add an initializer, make sure we don't introduce + a multiply by a pointer type, which can happen in certain cast + scenarios. */ + else if (!incr_vec[i].initializer + && TREE_CODE (first_dep->stride) != INTEGER_CST + && POINTER_TYPE_P (TREE_TYPE (first_dep->stride))) + + incr_vec[i].cost = COST_INFINITE; + /* For any other increment, if this is a multiply candidate, we must introduce a temporary T and initialize it with T_0 = stride * increment. When optimizing for speed, walk the