On Wed, Dec 14, 2016 at 01:39:13PM +0100, Dominik Vogt wrote: > There may be a slight imprecision in expand_compound_operation. > When it encounters a SIGN_EXTEND where it's already known that the > sign bit is zero, it may replace that with a ZERO_EXTEND (and > tries to simplify that further). However, the pattern is only > replaced if the new set_src_cost() is _lower_ than the old cost. > > The patch changes that to "not higher than", assuming that the > ZERO_EXTEND form is generally preferrable unless there is a reason > to believe it's not (i.e. its cost is higher). The comment atop > this code block seems to support this: > > /* Convert sign extension to zero extension, if we know that the high > bit is not set, as this is easier to optimize. It will be converted > back to cheaper alternative in make_extraction. */ > > On s390[x] this gets rid of some SIGN_EXTENDs completely. > > (The patched code uses the cheaper of both replacement patterns.)
That looks fine. But see below. > 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. Segher
