This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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


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.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]