Re: [PING^2][PATCH][2 of 2] RTL expansion for zero sign extension elimination with VRP

[Richard Biener <rguenther at suse dot de>]

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  <kuganv@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  <kuganv@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 <rguenther@suse.de>
SUSE / SUSE Labs
SUSE LINUX Products GmbH - Nuernberg - AG Nuernberg - HRB 16746
GF: Jeff Hawn, Jennifer Guild, Felix Imend