[Committed] Relax sizetype constraints in size_binop (take 2)

Richard Guenther richard.guenther@gmail.com
Sun Nov 12 13:56:00 GMT 2006


On 11/12/06, Roger Sayle <roger@eyesopen.com> wrote:
>
> The following patch is a revised version of my earlier submission
> http://gcc.gnu.org/ml/gcc-patches/2006-11/msg00142.html that implements
> Richard Guenther's suggestion of breaking the new "invariant" predicate
> out into it's own function.  The name of this new *middle-end private*
> function is int_binop_types_match_p, reflecting its role as the expected
> (required) type equality in calls to int_const_binop, and for integer
> operations to fold*.
>
> Missing from the following commit is the hunk:
>
> Index: fold-const.c
> ===================================================================
> *** fold-const.c        (revision 118681)
> --- fold-const.c        (working copy)
> *************** int_const_binop (enum tree_code code, tr
> *** 1393,1398 ****
> --- 1422,1433 ----
>       = (TREE_CODE (type) == INTEGER_TYPE && TYPE_IS_SIZETYPE (type));
>     int overflow = 0;
>
> + #if 0
> +   /* Temporarily disabled whilst the remaining type mismatches are fixed.  */
> +   gcc_assert (int_binop_types_match_p (code, TREE_TYPE (arg1),
> +                                        TREE_TYPE (arg2)));
> + #endif
> +
>     int1l = TREE_INT_CST_LOW (arg1);
>     int1h = TREE_INT_CST_HIGH (arg1);
>     int2l = TREE_INT_CST_LOW (arg2);
>
>
> which is the invariant that the middle-end expects to see hold between
> operands passed to const_int_binop.  Notice that const_int_binop doesn't
> take a type argument, so selects the operation type from the type of its
> arguments, arbitrarily arg1.  If the signedness of arg1 and arg2
> differ, it's not unlikely that these functions don't perform the
> operation that the caller intended.
>
> Naturally, the reason this hunk is omitted is that we currently pass
> completely bogus operands to fold* as revealed by Andrew Pinski's PR 22368
> meta-bug.  So although richi's comments suggested we were mysteriously
> weakening the middle-end's type system, in actuality the aim is to
> significantly strengthen it.  Asserting "type1 == type2" is incompatible
> with the tree-ssa/gimple notion of "useless_type_conversion", so we need
> something else.  Historically, matching modes was probably sufficient,
> but I suspect there are numerous latent bugs caused by type mismatching.
>
> Included in this commit are several more middle-end type correctness
> tweaks designed to get us closer to this goal of type safety.

This is definitely an improvement!  Thanks for doing this.  Now, I didn't like
int_const_binop and friends arbitrarily chosing one of the argument types
as the type of the result - do you think we should make the result type
explicit in these functions?  (I was thinking that, for the constant integer
nodes created by build_int_cst we might return constants with a canonicalized
type based on your new predicate if we don't want the result type of integer
operations explicitly specified)

We definitely have some "grey" area left here.  Does debug info at any time
change if we give constants more of a "type" than simply a mode?

Thanks,
Richard.



More information about the Gcc-patches mailing list