This is the mail archive of the
mailing list for the GCC project.
Re: [PING^2][PATCH][2 of 2] RTL expansion for zero sign extension elimination with VRP
- From: Richard Biener <rguenther at suse dot de>
- To: Kugan <kugan dot vivekanandarajah at linaro dot org>
- Cc: Eric Botcazou <ebotcazou at adacore dot com>, gcc-patches at gcc dot gnu dot org, steven at gcc dot gnu dot org
- Date: Fri, 18 Oct 2013 14:24:11 +0200 (CEST)
- Subject: Re: [PING^2][PATCH][2 of 2] RTL expansion for zero sign extension elimination with VRP
- Authentication-results: sourceware.org; auth=none
- References: <51ABFC6E dot 30205 at linaro dot org> <1726629 dot C2vZH2NXuZ at polaris> <520B31F5 dot 7020200 at linaro dot org> <52245B58 dot 6090507 at linaro dot org> <CAELXzTO=EdN_wn-EvYu6LALK8jLmfrDhwWNG_rRAK1xcXdPqWA at mail dot gmail dot com> <5253B4FA dot 9090203 at linaro dot org> <525D153A dot 1030808 at linaro dot org> <alpine dot LNX dot 2 dot 00 dot 1310151511420 dot 11149 at zhemvz dot fhfr dot qr> <525DDC09 dot 9090509 at linaro dot org>
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
> 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
> > 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.
> > [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 <email@example.com>
> + * 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,
> > Richard.
> >> Thanks,
> >> Kugan
> >>> +2013-09-25 Kugan Vivekanandarajah <firstname.lastname@example.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 <email@example.com>
SUSE / SUSE Labs
SUSE LINUX Products GmbH - Nuernberg - AG Nuernberg - HRB 16746
GF: Jeff Hawn, Jennifer Guild, Felix Imend