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][C-family] Fix PR61184


On Fri, 16 May 2014, Jeff Law wrote:

> On 05/14/14 03:06, Richard Biener wrote:
> > 
> > The following fixes pre/post-inc/dec gimplification of promoted
> > integer types.  There is the issue with the way TYPE_OVERFLOW_UNDEFINED
> > is related to TYPE_OVERFLOW_WRAPS and the (non-)semantics of
> > -fno-strict-overflow.
> > 
> > In this case, with -On -fno-strict-overflow for a variable of
> > type short we have !TYPE_OVERFLOW_WRAPS _and_ !TYPE_OVERFLOW_UNDEFINED
> > (so we're in an "undefined" area).  Which means that
> > !TYPE_OVERFLOW_UNDEFINED doesn't imply that overflow wraps.
> > 
> > Thus the gimplification has to play on the safe side and
> > always use an unsigned type unless the user specifies -fwrapv
> > (the flag with a proper semantic meaning).
> > 
> > That is, it seems to be the case that what predicate to use
> > (TYPE_OVERFLOW_WRAPS or TYPE_OVERFLOW_UNDEFINED, independent
> > on whether you invert it), depends on the use-case in a very
> > awkward (and error-prone) way.
> > 
> > Bootstrap and regtest pending on x86_64-unknown-linux-gnu, ok
> > if that succeeds (I expect to have to adjust some testcases)?
> > 
> > Thanks,
> > Richard.
> > 
> > 2014-05-14  Richard Biener  <rguenther@suse.de>
> > 
> > 	c-family/
> > 	* c-gimplify.c (c_gimplify_expr): Gimplify self-modify expressions
> > 	using unsigned arithmetic if overflow does not wrap instead of
> > 	if overflow is undefined.
> > 
> > 	* c-c++-common/torture/pr61184.c: New testcase.
> Seems reasonable to me.

If it is then I'd strongly suggest to make -fno-strict-overflow
imply -fwrapv.  Otherwise for -fno-strict-overflow we can
neither rely on signed arithmetic wrapping nor rely on it
invoking undefined behavior - instead it's in lala-land as far
as the middle-end is concerned (and get's us the worst of
both -fwrapv and -fno-wrapv).

Well, it turns out after re-visiting the bug, that the issue
lies in VRP instead as it doesn't detect [0, +INF] -> [0, +INF(OVF)]
as a lattice change and thus it misses visiting dependent
stmts again.

Bootstrap and regtest in progress on x86_64-unknown-linux-gnu.

Thanks,
Richard.

2014-05-19  Richard Biener  <rguenther@suse.de>

	PR tree-optimization/61184
	* tree-vrp.c (is_negative_overflow_infinity): Use
	TREE_OVERFLOW_P and do that check first.
	(is_positive_overflow_infinity): Likewise.
	(is_overflow_infinity): Likewise.
	(vrp_operand_equal_p): Properly treat operands with
	differing overflow as not equal.

	* c-c++-common/torture/pr61184.c: New testcase.

Index: gcc/tree-vrp.c
===================================================================
*** gcc/tree-vrp.c	(revision 210607)
--- gcc/tree-vrp.c	(working copy)
*************** positive_overflow_infinity (tree type)
*** 293,301 ****
  static inline bool
  is_negative_overflow_infinity (const_tree val)
  {
!   return (needs_overflow_infinity (TREE_TYPE (val))
! 	  && CONSTANT_CLASS_P (val)
! 	  && TREE_OVERFLOW (val)
  	  && vrp_val_is_min (val));
  }
  
--- 293,300 ----
  static inline bool
  is_negative_overflow_infinity (const_tree val)
  {
!   return (TREE_OVERFLOW_P (val)
! 	  && needs_overflow_infinity (TREE_TYPE (val))
  	  && vrp_val_is_min (val));
  }
  
*************** is_negative_overflow_infinity (const_tre
*** 304,312 ****
  static inline bool
  is_positive_overflow_infinity (const_tree val)
  {
!   return (needs_overflow_infinity (TREE_TYPE (val))
! 	  && CONSTANT_CLASS_P (val)
! 	  && TREE_OVERFLOW (val)
  	  && vrp_val_is_max (val));
  }
  
--- 303,310 ----
  static inline bool
  is_positive_overflow_infinity (const_tree val)
  {
!   return (TREE_OVERFLOW_P (val)
! 	  && needs_overflow_infinity (TREE_TYPE (val))
  	  && vrp_val_is_max (val));
  }
  
*************** is_positive_overflow_infinity (const_tre
*** 315,323 ****
  static inline bool
  is_overflow_infinity (const_tree val)
  {
!   return (needs_overflow_infinity (TREE_TYPE (val))
! 	  && CONSTANT_CLASS_P (val)
! 	  && TREE_OVERFLOW (val)
  	  && (vrp_val_is_min (val) || vrp_val_is_max (val)));
  }
  
--- 313,320 ----
  static inline bool
  is_overflow_infinity (const_tree val)
  {
!   return (TREE_OVERFLOW_P (val)
! 	  && needs_overflow_infinity (TREE_TYPE (val))
  	  && (vrp_val_is_min (val) || vrp_val_is_max (val)));
  }
  
*************** vrp_operand_equal_p (const_tree val1, co
*** 791,799 ****
      return true;
    if (!val1 || !val2 || !operand_equal_p (val1, val2, 0))
      return false;
!   if (is_overflow_infinity (val1))
!     return is_overflow_infinity (val2);
!   return true;
  }
  
  /* Return true, if the bitmaps B1 and B2 are equal.  */
--- 788,794 ----
      return true;
    if (!val1 || !val2 || !operand_equal_p (val1, val2, 0))
      return false;
!   return is_overflow_infinity (val1) == is_overflow_infinity (val2);
  }
  
  /* Return true, if the bitmaps B1 and B2 are equal.  */
Index: gcc/testsuite/c-c++-common/torture/pr61184.c
===================================================================
*** gcc/testsuite/c-c++-common/torture/pr61184.c	(revision 0)
--- gcc/testsuite/c-c++-common/torture/pr61184.c	(revision 0)
***************
*** 0 ****
--- 1,18 ----
+ /* { dg-do run } */
+ /* { dg-additional-options "-fno-strict-overflow" } */
+ 
+ short a; 
+ 
+ void
+ foo (void)
+ {
+   for (a = 0; a >= 0; a++)
+     ;
+ }
+ 
+ int
+ main ()
+ {
+   foo ();
+   return 0;
+ }


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