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: [PATCH] Fix PR26998, bad logic in VRP for NEGATE_EXPR


On Fri, 9 Jun 2006, Duncan Sands wrote:

> > This fixes PR26998 by ensuring that we do not use fold_unary_to_constant
> > with a NEGATE_EXPR if we know that will overflow.  The current check for
> > this case is bogus, as the only value that will overflow in this case
> > is the signed TYPE_MIN_VALUE, there is no reason to handle TYPE_MAX_VALUE
> > specially.
> 
> Ada can generate types with pretty much any value for TYPE_MIN_VALUE
> and TYPE_MAX_VALUE, for example TYPE_MIN_VALUE=-1, TYPE_MAX_VALUE=10.
> Isn't -TYPE_MAX_VALUE then problematic (not to mention -2, .., -9)?

Yes, but that doesn't change before or after the patch.  In fact, VRP
has enough places that assume TYPE_MIN/MAX_VALUE behave this way.  I also
doubt that fold_unary (NEGATE_EXPR, -8) does something sensible if
presented with a type with a range of [-8, 1] for example (I bet we
either get -8 or 0 out of it).  So, to be safe, vrp_visit_assignment
should not record ranges for types that are problematic.  The fold
range stuff contains an explicit check for that:

      /* Check if (unsigned) INT_MAX + 1 == (unsigned) INT_MIN
         for the type in question, as we rely on this here.  */
      utype = lang_hooks.types.unsigned_type (etype);
      maxv = fold_convert (utype, TYPE_MAX_VALUE (etype));
      maxv = range_binop (PLUS_EXPR, NULL_TREE, maxv, 1,
                          integer_one_node, 1);
      minv = fold_convert (utype, TYPE_MIN_VALUE (etype));

      if (integer_zerop (range_binop (NE_EXPR, integer_type_node,
                                      minv, 1, maxv, 1)))
        etype = utype;
      else
        return 0;

But I think any fix in this direction is orthogonal to the patch for
26998 (and Ada is not release critical and the patch tested fine with 
Ada).

Thanks,
Richard.

--
Richard Guenther <rguenther@suse.de>
Novell / SUSE Labs


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