Re: [Committed] Preserve TREE_OVERFLOW in size_binop

["Roger Sayle" <roger at eyesopen dot com>]

Hi Richard,

On Sun, January 28, 2007 9 am, Richard Guenther wrote:
> Hmm, I fail to see why we can have extra overflow here:
>
> ! 	  t = build_int_cst_wide_type (TREE_TYPE (value), low, high);
> ! 	  if ((TREE_OVERFLOW (value) || integer_zerop (t))
> ! 	      && !TREE_OVERFLOW (t))
> ! 	    {
> ! 	      t = copy_node (t);
> ! 	      TREE_OVERFLOW (t) = 1;
> ! 	    }
> ! 	  return t;
>
> TREE_OVERFLOW (t) will always be false, as we round up and know
> divisor is > 0 integer_zerop (t) should also be false, so the only case
> we need to care about is propagating TREE_OVERFLOW (value)
> for which you can use
> return force_fit_type_double (TREE_TYPE (value), low, high, 0,
> TREE_OVERFLOW (value));

There are usually always three sources of overflow that we need to worry
about in the middle-end;

[1] Preserving the TREE_OVERFLOW bits from our operands.
[2] The overflow that results from being unable to represent the high/low
double word in the destination type.
[3] The overflow that's caused from the result of any arithmetic
operations not being representable in a high/low double word.

In your analysis above you consider the first two cases, but neglect the
third.  When asked to round up 0xffffffff:0xffffffff to a multiple of
four, the result ends up being unrepresentable with the value zero, hence
the integer_zerop test.  Notice that because the code has already
efficiently handled the already rounded cases, a zero on input never
reaches here, so a zero on output is our overflow indicator.  You'll
notice that the same three cases are handled in int_const_binop and
elsewhere in fold-const.c, where for example the return value of
add_double tracks the overflow in case [3], and then force_fit_type_double
handles case [2], which is usually passed the disjunction of TREE_OVERFLOW
of the operands, handling case [1].

However, you're right that I can use the new force_fit_type_double API to
more efficiently acheive this by passing in an overflow indicator, rather
than copying the node and setting TREE_OVERFLOW afterwards (as we used to
do with force_fit_type).  I'm currently bootstraping and regression
testing a follow-up patch to implement that tweak.

Thanks,

Roger
--