On Thu, Dec 15, 2016 at 01:57:06PM +0100, Dominik Vogt wrote:
> > > The patch hasn't got a lot of testing yet as I'd like to hear your
> > > opinion on the patch first.
> >
> > I am testing it on powerpc. Please also test on x86?
> >
> > > gcc/ChangeLog-signextend-1
> > >
> > > * combine.c (expand_compound_operation): Substitute ZERO_EXTEND for
> > > SIGN_EXTEND if the costs are equal or lower.
> > > Choose the cheapest replacement.
> >
> > > /* Make sure this is a profitable operation. */
> > > if (set_src_cost (x, mode, optimize_this_for_speed_p)
> > > - > set_src_cost (temp2, mode, optimize_this_for_speed_p))
> > > - return temp2;
> > > - else if (set_src_cost (x, mode, optimize_this_for_speed_p)
> > > - > set_src_cost (temp, mode, optimize_this_for_speed_p))
> > > - return temp;
> > > - else
> > > - return x;
> > > + >= set_src_cost (temp2, mode, optimize_this_for_speed_p))
> > > + x = temp2;
> > > + if (set_src_cost (x, mode, optimize_this_for_speed_p)
> > > + >= set_src_cost (temp, mode, optimize_this_for_speed_p))
> > > + x = temp;
> > > + return x;
> > > }
> >
> > So this prefers the zero_extend version over the expand_compound_operation
> > version, I wonder if that is a good idea.
>
> Maybe this is a little less disruptive:
>
> int ctemp = set_src_cost (temp, mode, optimize_this_for_speed_p);
> int ctemp2 = set_src_cost (temp2, mode, optimize_this_for_speed_p);
>
> /* Make sure this is a profitable operation. */
> if (MIN (ctemp, ctemp2)
> <= set_src_cost (x, mode, optimize_this_for_speed_p))
> x = (ctemp < ctemp2) ? temp : temp2;
> return x;
Or just swap the temp and temp2 cases in your original patch. Which btw
tested fine on powerpc64-linux {-m32,-m64}.
Segher
