Re: [PATCH] Fix PR35634

Re: [PATCH] Fix PR35634

[David Edelsohn]

Richard,

The testcases assume default signed char and fail on systems with
different semantics.  I believe that both testcases need to declare c
as signed char to consistently test the desired behavior, right?

Thanks, David

Re: [PATCH] Fix PR35634

[Richard Biener]

On Sat, 1 Dec 2012, David Edelsohn wrote:

> Richard,
> 
> The testcases assume default signed char and fail on systems with
> different semantics.  I believe that both testcases need to declare c
> as signed char to consistently test the desired behavior, right?

Fixed as follows.

Richard.

2012-12-03  Richard Biener  <rguenther@suse.de>

	* gcc.dg/torture/pr35634.c: Use signed char.
	* g++.dg/torture/pr35634.C: Likewise.

Index: gcc/testsuite/gcc.dg/torture/pr35634.c
===================================================================
--- gcc/testsuite/gcc.dg/torture/pr35634.c	(revision 194077)
+++ gcc/testsuite/gcc.dg/torture/pr35634.c	(working copy)
@@ -14,6 +14,6 @@ void foo (int i)
 
 int main ()
 {
-    char c;
+    signed char c;
     for (c = 0; ; c++) foo (c);
 }
Index: gcc/testsuite/g++.dg/torture/pr35634.C
===================================================================
--- gcc/testsuite/g++.dg/torture/pr35634.C	(revision 194077)
+++ gcc/testsuite/g++.dg/torture/pr35634.C	(working copy)
@@ -14,6 +14,6 @@ void foo (int i)
 
 int main ()
 {
-    char c;
+    signed char c;
     for (c = 0; ; c++) foo (c);
 }

Re: [PATCH] Fix PR35634

[Joseph S. Myers]

On Tue, 27 Nov 2012, Richard Biener wrote:

> 	c-family/
> 	* c-gimplify.c (c_gimplify_expr): Gimplify self-modify expressions
> 	here and use a type with proper overflow behavior for types that would
> 	need to be promoted for the arithmetic.

The front-end changes are OK.

> *** gcc/gimplify.c.orig	2012-11-27 14:27:24.000000000 +0100
> --- gcc/gimplify.c	2012-11-27 15:49:46.425585857 +0100
> *************** gimplify_compound_lval (tree *expr_p, gi
> *** 2319,2327 ****
>       WANT_VALUE is nonzero iff we want to use the value of this expression
>   	in another expression.  */
>   
> ! static enum gimplify_status
>   gimplify_self_mod_expr (tree *expr_p, gimple_seq *pre_p, gimple_seq *post_p,
> ! 			bool want_value)
>   {
>     enum tree_code code;
>     tree lhs, lvalue, rhs, t1;
> --- 2319,2327 ----
>       WANT_VALUE is nonzero iff we want to use the value of this expression
>   	in another expression.  */
>   
> ! enum gimplify_status
>   gimplify_self_mod_expr (tree *expr_p, gimple_seq *pre_p, gimple_seq *post_p,
> ! 			bool want_value, tree arith_type)

The comment needs updating to explain the new parameter.

-- 
Joseph S. Myers
joseph@codesourcery.com

[PATCH] Fix PR35634

[Richard Biener]

This re-surrects Josephs patch to fix PR35634 - the fact that
the C frontend family does not properly promote the expression
operands of self-modify expressions.  Accounting for this during
gimplification is indeed easiest (as long as fold isn't too clever
with the expressions in unpromoted form).  The patch, instead of
promoting to int/unsigned int uses an unsigned type for the
computation where otherwise undefined behavior would arise from
the computation in a type with lower precision.  That avoids
most of the vectorizer regressions with the earlier patch.
The remaining three regressions are fixed with the
vect_can_advance_ivs_p hunk.

Bootstrapped and tested on x86_64-unknown-linux-gnu without
the vect_can_advance_ivs_p hunk, re-testing with that.

Ok for trunk?  (I don't think this kind of patch qualifies for
backporting)

Thanks,
Richard.

2008-04-17  Richard Guenther  <rguenther@suse.de>

	PR c/35634
	* gimple.h (gimplify_self_mod_expr): Declare.
	* gimplify.c (gimplify_self_mod_expr): Export.  Take a different
	type for performing the arithmetic in.
	(gimplify_expr): Adjust.
	* tree-vect-loop-manip.c (vect_can_advance_ivs_p): Strip
	sign conversions we can re-apply after adjusting the IV.

	c-family/
	* c-gimplify.c (c_gimplify_expr): Gimplify self-modify expressions
	here and use a type with proper overflow behavior for types that would
	need to be promoted for the arithmetic.

	* gcc.dg/torture/pr35634.c: New testcase.
	* g++.dg/torture/pr35634.C: Likewise.
	* gcc.dg/vect/pr18536.c: Mark worker function noinline.

Index: gcc/testsuite/g++.dg/torture/pr35634.C
===================================================================
*** /dev/null	1970-01-01 00:00:00.000000000 +0000
--- gcc/testsuite/g++.dg/torture/pr35634.C	2012-11-27 15:49:46.387585847 +0100
***************
*** 0 ****
--- 1,19 ----
+ /* { dg-do run } */
+ 
+ extern "C" void abort (void);
+ extern "C" void exit (int);
+ 
+ void foo (int i)
+ {
+     static int n;
+     if (i < -128 || i > 127)
+         abort ();
+     if (++n > 1000)
+         exit (0);
+ }
+ 
+ int main ()
+ {
+     char c;
+     for (c = 0; ; c++) foo (c);
+ }
Index: gcc/testsuite/gcc.dg/torture/pr35634.c
===================================================================
*** /dev/null	1970-01-01 00:00:00.000000000 +0000
--- gcc/testsuite/gcc.dg/torture/pr35634.c	2012-11-27 15:49:46.424585859 +0100
***************
*** 0 ****
--- 1,19 ----
+ /* { dg-do run } */
+ 
+ void abort (void);
+ void exit (int);
+ 
+ void foo (int i)
+ {
+     static int n;
+     if (i < -128 || i > 127)
+         abort ();
+     if (++n > 1000)
+         exit (0);
+ }
+ 
+ int main ()
+ {
+     char c;
+     for (c = 0; ; c++) foo (c);
+ }
Index: gcc/gimple.h
===================================================================
*** gcc/gimple.h.orig	2012-11-27 14:27:24.000000000 +0100
--- gcc/gimple.h	2012-11-27 15:49:46.424585859 +0100
*************** extern enum gimplify_status gimplify_exp
*** 979,984 ****
--- 979,986 ----
  					   bool (*) (tree), fallback_t);
  extern void gimplify_type_sizes (tree, gimple_seq *);
  extern void gimplify_one_sizepos (tree *, gimple_seq *);
+ enum gimplify_status gimplify_self_mod_expr (tree *, gimple_seq *, gimple_seq *,
+ 					     bool, tree);
  extern bool gimplify_stmt (tree *, gimple_seq *);
  extern gimple gimplify_body (tree, bool);
  extern void push_gimplify_context (struct gimplify_ctx *);
Index: gcc/gimplify.c
===================================================================
*** gcc/gimplify.c.orig	2012-11-27 14:27:24.000000000 +0100
--- gcc/gimplify.c	2012-11-27 15:49:46.425585857 +0100
*************** gimplify_compound_lval (tree *expr_p, gi
*** 2319,2327 ****
      WANT_VALUE is nonzero iff we want to use the value of this expression
  	in another expression.  */
  
! static enum gimplify_status
  gimplify_self_mod_expr (tree *expr_p, gimple_seq *pre_p, gimple_seq *post_p,
! 			bool want_value)
  {
    enum tree_code code;
    tree lhs, lvalue, rhs, t1;
--- 2319,2327 ----
      WANT_VALUE is nonzero iff we want to use the value of this expression
  	in another expression.  */
  
! enum gimplify_status
  gimplify_self_mod_expr (tree *expr_p, gimple_seq *pre_p, gimple_seq *post_p,
! 			bool want_value, tree arith_type)
  {
    enum tree_code code;
    tree lhs, lvalue, rhs, t1;
*************** gimplify_self_mod_expr (tree *expr_p, gi
*** 2382,2408 ****
  	return ret;
      }
  
    /* For POINTERs increment, use POINTER_PLUS_EXPR.  */
    if (POINTER_TYPE_P (TREE_TYPE (lhs)))
      {
        rhs = convert_to_ptrofftype_loc (loc, rhs);
        if (arith_code == MINUS_EXPR)
  	rhs = fold_build1_loc (loc, NEGATE_EXPR, TREE_TYPE (rhs), rhs);
!       arith_code = POINTER_PLUS_EXPR;
      }
  
    if (postfix)
      {
-       tree t2 = get_initialized_tmp_var (lhs, pre_p, NULL);
-       t1 = build2 (arith_code, TREE_TYPE (*expr_p), t2, rhs);
        gimplify_assign (lvalue, t1, pre_p);
        gimplify_seq_add_seq (orig_post_p, post);
!       *expr_p = t2;
        return GS_ALL_DONE;
      }
    else
      {
-       t1 = build2 (arith_code, TREE_TYPE (*expr_p), lhs, rhs);
        *expr_p = build2 (MODIFY_EXPR, TREE_TYPE (lvalue), lvalue, t1);
        return GS_OK;
      }
--- 2382,2413 ----
  	return ret;
      }
  
+   if (postfix)
+     lhs = get_initialized_tmp_var (lhs, pre_p, NULL);
+ 
    /* For POINTERs increment, use POINTER_PLUS_EXPR.  */
    if (POINTER_TYPE_P (TREE_TYPE (lhs)))
      {
        rhs = convert_to_ptrofftype_loc (loc, rhs);
        if (arith_code == MINUS_EXPR)
  	rhs = fold_build1_loc (loc, NEGATE_EXPR, TREE_TYPE (rhs), rhs);
!       t1 = fold_build2 (POINTER_PLUS_EXPR, TREE_TYPE (*expr_p), lhs, rhs);
      }
+   else
+     t1 = fold_convert (TREE_TYPE (*expr_p),
+ 		       fold_build2 (arith_code, arith_type,
+ 				    fold_convert (arith_type, lhs),
+ 				    fold_convert (arith_type, rhs)));
  
    if (postfix)
      {
        gimplify_assign (lvalue, t1, pre_p);
        gimplify_seq_add_seq (orig_post_p, post);
!       *expr_p = lhs;
        return GS_ALL_DONE;
      }
    else
      {
        *expr_p = build2 (MODIFY_EXPR, TREE_TYPE (lvalue), lvalue, t1);
        return GS_OK;
      }
*************** gimplify_expr (tree *expr_p, gimple_seq
*** 7111,7117 ****
  	case PREINCREMENT_EXPR:
  	case PREDECREMENT_EXPR:
  	  ret = gimplify_self_mod_expr (expr_p, pre_p, post_p,
! 					fallback != fb_none);
  	  break;
  
  	case ARRAY_REF:
--- 7116,7123 ----
  	case PREINCREMENT_EXPR:
  	case PREDECREMENT_EXPR:
  	  ret = gimplify_self_mod_expr (expr_p, pre_p, post_p,
! 					fallback != fb_none,
! 					TREE_TYPE (*expr_p));
  	  break;
  
  	case ARRAY_REF:
Index: gcc/c-family/c-gimplify.c
===================================================================
*** gcc/c-family/c-gimplify.c.orig	2012-11-27 14:27:24.000000000 +0100
--- gcc/c-family/c-gimplify.c	2012-11-27 15:50:08.796585070 +0100
*************** c_gimplify_expr (tree *expr_p, gimple_se
*** 172,187 ****
  {
    enum tree_code code = TREE_CODE (*expr_p);
  
!   /* This is handled mostly by gimplify.c, but we have to deal with
!      not warning about int x = x; as it is a GCC extension to turn off
!      this warning but only if warn_init_self is zero.  */
!   if (code == DECL_EXPR
!       && TREE_CODE (DECL_EXPR_DECL (*expr_p)) == VAR_DECL
!       && !DECL_EXTERNAL (DECL_EXPR_DECL (*expr_p))
!       && !TREE_STATIC (DECL_EXPR_DECL (*expr_p))
!       && (DECL_INITIAL (DECL_EXPR_DECL (*expr_p)) == DECL_EXPR_DECL (*expr_p))
!       && !warn_init_self)
!     TREE_NO_WARNING (DECL_EXPR_DECL (*expr_p)) = 1;
  
    return GS_UNHANDLED;
  }
--- 172,208 ----
  {
    enum tree_code code = TREE_CODE (*expr_p);
  
!   switch (code)
!     {
!     case DECL_EXPR:
!       /* This is handled mostly by gimplify.c, but we have to deal with
! 	 not warning about int x = x; as it is a GCC extension to turn off
! 	 this warning but only if warn_init_self is zero.  */
!       if (TREE_CODE (DECL_EXPR_DECL (*expr_p)) == VAR_DECL
! 	  && !DECL_EXTERNAL (DECL_EXPR_DECL (*expr_p))
! 	  && !TREE_STATIC (DECL_EXPR_DECL (*expr_p))
! 	  && (DECL_INITIAL (DECL_EXPR_DECL (*expr_p)) == DECL_EXPR_DECL (*expr_p))
! 	  && !warn_init_self)
! 	TREE_NO_WARNING (DECL_EXPR_DECL (*expr_p)) = 1;
!       break;
! 
!     case PREINCREMENT_EXPR:
!     case PREDECREMENT_EXPR:
!     case POSTINCREMENT_EXPR:
!     case POSTDECREMENT_EXPR:
!       {
! 	tree type = TREE_TYPE (TREE_OPERAND (*expr_p, 0));
! 	if (INTEGRAL_TYPE_P (type) && c_promoting_integer_type_p (type))
! 	  {
! 	    if (TYPE_OVERFLOW_UNDEFINED (type))
! 	      type = unsigned_type_for (type);
! 	    return gimplify_self_mod_expr (expr_p, pre_p, post_p, 1, type);
! 	  }
! 	break;
!       }
! 
!     default:;
!     }
  
    return GS_UNHANDLED;
  }
Index: gcc/testsuite/gcc.dg/vect/pr18536.c
===================================================================
*** gcc/testsuite/gcc.dg/vect/pr18536.c.orig	2006-02-07 11:14:12.000000000 +0100
--- gcc/testsuite/gcc.dg/vect/pr18536.c	2012-11-27 15:59:50.510564950 +0100
***************
*** 5,11 ****
  
  #define N 16
  
! int main1 (short a, short *b)
  {
    while (++a < 4) *b++ = 2;
  
--- 5,11 ----
  
  #define N 16
  
! __attribute__ ((noinline)) int main1 (short a, short *b)
  {
    while (++a < 4) *b++ = 2;
  
Index: gcc/tree-vect-loop-manip.c
===================================================================
*** gcc/tree-vect-loop-manip.c.orig	2012-11-26 12:40:21.000000000 +0100
--- gcc/tree-vect-loop-manip.c	2012-11-27 16:10:37.601542471 +0100
*************** vect_can_advance_ivs_p (loop_vec_info lo
*** 1727,1732 ****
--- 1727,1733 ----
  	  return false;
  	}
  
+       STRIP_NOPS (access_fn);
        if (dump_enabled_p ())
          {
  	  dump_printf_loc (MSG_NOTE, vect_location,