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, 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). 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). [exposing the effect of PROMOTE_MODE earlier than at RTL expansion time may make this less awkward] 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. > > + > > > >
