On Wed, 16 Oct 2013, Kugan wrote: > Thanks Richard for the review. > > On 15/10/13 23:55, Richard Biener wrote: > > On Tue, 15 Oct 2013, Kugan wrote: > > > >> Hi Eric, > >> > >> Can you please help to review this patch? > >> http://gcc.gnu.org/ml/gcc-patches/2013-10/msg00452.html > > > > I think that gimple_assign_is_zero_sign_ext_redundant and its > > description is somewhat confused. You seem to have two cases > > here, one being NOP_EXPR or CONVERT_EXPR that are in the IL > > because they are required for type correctness. So, > > > > I have changed the name and the comments to make it more clear. > > > long = (long) int > > > > which usually expands to > > > > (set:DI (sext:DI reg:SI)) > > > > you want to expand as > > > > (set:DI (subreg:DI reg:SI)) > > > > where you have to be careful for modes smaller than word_mode. > > You don't seem to implement this optimization though (but > > the gimple_assign_is_zero_sign_ext_redundant talks about it). > > > > > I am actually handling only the cases smaller than word_mode. If there > is any sign change, I dont do any changes. In the place RTL expansion is > done, I have added these condition as it is done for convert_move and > others. > > For example, when an expression is evaluated and it's value is assigned > to variable of type short, the generated RTL would look something like > the following. > > (set (reg:SI 110) > (zero_extend:SI (subreg:HI (reg:SI 117) 0))) > > However, if during value range propagation, if we can say for certain > that the value of the expression which is present in register 117 is > within the limits of short and there is no sign conversion, we do not > need to perform the subreg and zero_extend; instead we can generate the > following RTl. > > (set (reg:SI 110) > (reg:SI 117))) > > > > Second are promotions required by the target (PROMOTE_MODE) > > that do arithmetic on wider registers like for > > > > char = char + char > > > > where they cannot do > > > > (set:QI (plus:QI reg:QI reg:QI)) > > > > but instead have, for example reg:SI only and you get > > > > (set:SI (plus:SI reg:SI reg:SI)) > > (set:SI ([sz]ext:SI (subreg:QI reg:SI))) > > > > that you try to address with the cfgexpand hunk but I believe > > it doesn't work the way you do it. That is because on GIMPLE > > you do not see SImode temporaries and thus no SImode value-ranges. > > Consider > > > > tem = (char) 255 + (char) 1; > > > > which has a value-range of [0,0] but clearly when computed in > > SImode the value-range is [256, 256]. That is, VRP computes > > value-ranges in the expression type, not in some arbitrary > > larger type. > > > > So what you'd have to do is take the value-ranges of the > > two operands of the plus and see whether the plus can overflow > > QImode when computed in SImode (for the example). > > > > Yes. Instead of calculating the value ranges of the two operand in > SImode, What I am doing in this case is to look at the value range of > tem and if it is within [CHAR_MIN + 1, CHAR_MAX -1]. As you have > explained earlier, we cant rely on being within the [CHAR_MIN, CHAR_MAX] > as the range could have been modified to fit the LHS type. This ofcourse > will miss some of the cases where we can remove extensions but > simplifies the logic.
Not sure if I understand what you are saying here. As for the above case > > tem = (char) 255 + (char) 1; tem is always of type 'char' in GIMPLE (even if later promoted via PROMOTE_MODE) the value-range is a 'char' value-range and thus never will exceed [CHAR_MIN, CHAR_MAX]. The only way you can use that directly is if you can rely on undefined behavior happening for signed overflow - but if you argue that way you can simply _always_ drop the (sext:SI (subreg:QI part and you do not need value ranges for this. For unsigned operations for example [250, 254] + [8, 10] will simply wrap to [3, 7] (if I got the math correct) which is inside your [CHAR_MIN + 1, CHAR_MAX - 1] but if performed in SImode you can get 259 and thus clearly you cannot drop the (zext:SI (subreg:QI parts. The same applies to signed types if you do not want to rely on signed overflow being undefined of course. Richard. > > [exposing the effect of PROMOTE_MODE earlier than at RTL expansion > > time may make this less awkward] > > Please find the modified patch attached. > > > +2013-10-16 Kugan Vivekanandarajah <kug...@linaro.org> > + > + * dojump.c (do_compare_and_jump): Generate rtl without > + zero/sign extension if redundant. > + * cfgexpand.c (expand_gimple_stmt_1): Likewise. > + * gimple.c (gimple_is_rhs_value_fits_in_assign) : New > + function. > + * gimple.h (gimple_is_rhs_value_fits_in_assign) : Declare. > + > > Thanks, > Kugan > > > > Thanks, > > Richard. > > > >> Thanks, > >> Kugan > >> > >>> +2013-09-25 Kugan Vivekanandarajah <kug...@linaro.org> > >>> + > >>> + * dojump.c (do_compare_and_jump): Generate rtl without > >>> + zero/sign extension if redundant. > >>> + * cfgexpand.c (expand_gimple_stmt_1): Likewise. > >>> + * gimple.c (gimple_assign_is_zero_sign_ext_redundant) : New > >>> + function. > >>> + * gimple.h (gimple_assign_is_zero_sign_ext_redundant) : Declare. > >>> + > >>> > >>> > > -- Richard Biener <rguent...@suse.de> SUSE / SUSE Labs SUSE LINUX Products GmbH - Nuernberg - AG Nuernberg - HRB 16746 GF: Jeff Hawn, Jennifer Guild, Felix Imend
