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: [C PATCH] Discard P - (P + CST) optimization in pointer_diff (PR c/61240)


On Mon, Aug 04, 2014 at 02:04:36PM +0200, Richard Biener wrote:
> > Looks like .fre can optimize "q - (q - 1)" into 1:
> >    <bb 2>:
> >    q.0_3 = (long int) &MEM[(void *)&i + 4B];
> >    _5 = (long int) &i;
> > -  _6 = q.0_3 - _5;
> > -  t.1_7 = _6 /[ex] 4;
> > -  t ={v} t.1_7;
> > +  t ={v} 1;
> >    i ={v} {CLOBBER};
> >    return;
> >  
> > But associate_plusminus doesn't optimize it:
> >           else if (code == MINUS_EXPR
> >                    && CONVERT_EXPR_CODE_P (def_code)
> >                    && TREE_CODE (gimple_assign_rhs1 (def_stmt)) == SSA_NAME
> >                    && TREE_CODE (rhs2) == SSA_NAME)
> >             {
> >               /* (T)(P + A) - (T)P -> (T)A.  */
> > becase gimple_assign_rhs1 (def_stmt) is not an SSA_NAME, but ADDR_EXPR (it's
> > &MEM[(void *)&i + 4B]).  Then there's transformation A - (A +- B) -> -+ B
> > below, but that doesn't handle casts.
> > 
> > So - should I try to handle it in associate_plusminus?
> 
> Yes please, with a (few) testcase(s).

Ok, so the following adds the (T)P - (T)(P + A) -> (T)-A
transformation.  It is based on code hunk that does the 
(T)(P + A) - (T)P -> (T)A transformation.  The difference it makes is
in the .optimized dump something like:

 int fn2(int, int) (int p, int i)
 {
-  unsigned int p.2_2;
+  unsigned int _3;
   int _4;
-  unsigned int _5;
   unsigned int _6;
-  int _7;
 
   <bb 2>:
-  p.2_2 = (unsigned int) p_1(D);
-  _4 = p_1(D) + i_3(D);
-  _5 = (unsigned int) _4;
-  _6 = p.2_2 - _5;
-  _7 = (int) _6;
-  return _7;
+  _6 = (unsigned int) i_2(D);
+  _3 = -_6;
+  _4 = (int) _3;
+  return _4;

i.e., the PLUS_EXPR and MINUS_EXPR are gone, and NEGATE_EXPR is used instead.
During bootstrap with --enable-languages=c,c++ this optimization triggered 238 times.

Bootstrapped/regtested on x86_64-linux, ok for trunk?

2014-08-05  Marek Polacek  <polacek@redhat.com>

	PR c/61240
	* tree-ssa-forwprop.c (associate_plusminus): Add (T)P - (T)(P + A)
	-> (T)-A transformation.
c/
	* c-typeck.c (pointer_diff): Remove P - (P + CST) optimization.
testsuite/
	* gcc.dg/pr61240.c: New test.
	* gcc.dg/tree-ssa/forwprop-29.c: New test.

diff --git gcc/c/c-typeck.c gcc/c/c-typeck.c
index fe9440c..5f46944 100644
--- gcc/c/c-typeck.c
+++ gcc/c/c-typeck.c
@@ -3460,7 +3460,6 @@ pointer_diff (location_t loc, tree op0, tree op1)
   addr_space_t as0 = TYPE_ADDR_SPACE (TREE_TYPE (TREE_TYPE (op0)));
   addr_space_t as1 = TYPE_ADDR_SPACE (TREE_TYPE (TREE_TYPE (op1)));
   tree target_type = TREE_TYPE (TREE_TYPE (op0));
-  tree con0, con1, lit0, lit1;
   tree orig_op1 = op1;
 
   /* If the operands point into different address spaces, we need to
@@ -3490,7 +3489,6 @@ pointer_diff (location_t loc, tree op0, tree op1)
   else
     inttype = restype;
 
-
   if (TREE_CODE (target_type) == VOID_TYPE)
     pedwarn (loc, OPT_Wpointer_arith,
 	     "pointer of type %<void *%> used in subtraction");
@@ -3498,50 +3496,6 @@ pointer_diff (location_t loc, tree op0, tree op1)
     pedwarn (loc, OPT_Wpointer_arith,
 	     "pointer to a function used in subtraction");
 
-  /* If the conversion to ptrdiff_type does anything like widening or
-     converting a partial to an integral mode, we get a convert_expression
-     that is in the way to do any simplifications.
-     (fold-const.c doesn't know that the extra bits won't be needed.
-     split_tree uses STRIP_SIGN_NOPS, which leaves conversions to a
-     different mode in place.)
-     So first try to find a common term here 'by hand'; we want to cover
-     at least the cases that occur in legal static initializers.  */
-  if (CONVERT_EXPR_P (op0)
-      && (TYPE_PRECISION (TREE_TYPE (op0))
-	  == TYPE_PRECISION (TREE_TYPE (TREE_OPERAND (op0, 0)))))
-    con0 = TREE_OPERAND (op0, 0);
-  else
-    con0 = op0;
-  if (CONVERT_EXPR_P (op1)
-      && (TYPE_PRECISION (TREE_TYPE (op1))
-	  == TYPE_PRECISION (TREE_TYPE (TREE_OPERAND (op1, 0)))))
-    con1 = TREE_OPERAND (op1, 0);
-  else
-    con1 = op1;
-
-  if (TREE_CODE (con0) == POINTER_PLUS_EXPR)
-    {
-      lit0 = TREE_OPERAND (con0, 1);
-      con0 = TREE_OPERAND (con0, 0);
-    }
-  else
-    lit0 = integer_zero_node;
-
-  if (TREE_CODE (con1) == POINTER_PLUS_EXPR)
-    {
-      lit1 = TREE_OPERAND (con1, 1);
-      con1 = TREE_OPERAND (con1, 0);
-    }
-  else
-    lit1 = integer_zero_node;
-
-  if (operand_equal_p (con0, con1, 0))
-    {
-      op0 = lit0;
-      op1 = lit1;
-    }
-
-
   /* First do the subtraction as integers;
      then drop through to build the divide operator.
      Do not do default conversions on the minus operator
diff --git gcc/testsuite/gcc.dg/pr61240.c gcc/testsuite/gcc.dg/pr61240.c
index e69de29..115e415 100644
--- gcc/testsuite/gcc.dg/pr61240.c
+++ gcc/testsuite/gcc.dg/pr61240.c
@@ -0,0 +1,13 @@
+/* PR c/61240 */
+/* { dg-do compile } */
+
+void
+foo (void)
+{
+  volatile __PTRDIFF_TYPE__ t;
+  int i;
+  int *p = &i;
+  int *q = &i + 1;
+  t = q - (q - 1);
+  t = p - (p - 1);
+}
diff --git gcc/testsuite/gcc.dg/tree-ssa/forwprop-29.c gcc/testsuite/gcc.dg/tree-ssa/forwprop-29.c
index e69de29..745a494 100644
--- gcc/testsuite/gcc.dg/tree-ssa/forwprop-29.c
+++ gcc/testsuite/gcc.dg/tree-ssa/forwprop-29.c
@@ -0,0 +1,85 @@
+/* { dg-do run } */
+/* { dg-options "-O -fdump-tree-forwprop1" } */
+
+__PTRDIFF_TYPE__ __attribute__((noinline, noclone))
+fn1 (int *p, long int i)
+{
+  return p - (p + i);
+}
+
+int __attribute__((noinline, noclone))
+fn2 (int p, int i)
+{
+  return (long) p - (p + i);
+}
+
+long int __attribute__((noinline, noclone))
+fn3 (long int c, long int i)
+{
+  return (unsigned long) c - (c + i);
+}
+
+int
+main ()
+{
+  int l = 10;
+  if (fn1 (&l, 1) != -1
+      || fn1 (&l, -1) != 1
+      || fn1 (&l, 0) != 0
+      || fn1 (&l, 0x186A0) != -0x186A0
+      || fn1 (&l, 0xb3c2) != -0xb3c2
+      || fn1 (&l, 0xb3) != -0xb3
+      || fn1 (&l, 0xacf) != -0xacf
+      || fn1 (&l, 0xf) != -0xf
+      || fn1 (&l, 0x2468acf) != -0x2468acf
+      || fn1 (&l, 0x91a2b3c) != -0x91a2b3c
+      || fn1 (&l, -0x186A0) != 0x186A0
+      || fn1 (&l, -0xb3c2) != 0xb3c2
+      || fn1 (&l, -0xb3) != 0xb3
+      || fn1 (&l, -0xacf) != 0xacf
+      || fn1 (&l, -0xf) != 0xf
+      || fn1 (&l, -0x2468acf) != 0x2468acf
+      || fn1 (&l, -0x91a2b3c) != 0x91a2b3c)
+    __builtin_abort ();
+  if (fn2 (l, 1) != -1
+      || fn2 (l, -1) != 1
+      || fn2 (l, 0) != 0
+      || fn2 (l, 0x186A0) != -0x186A0
+      || fn2 (l, 0xb3c2) != -0xb3c2
+      || fn2 (l, 0xb3) != -0xb3
+      || fn2 (l, 0xacf) != -0xacf
+      || fn2 (l, 0xf) != -0xf
+      || fn2 (l, 0x2468acf) != -0x2468acf
+      || fn2 (l, 0x91a2b3c) != -0x91a2b3c
+      || fn2 (l, -0x186A0) != 0x186A0
+      || fn2 (l, -0xb3c2) != 0xb3c2
+      || fn2 (l, -0xb3) != 0xb3
+      || fn2 (l, -0xacf) != 0xacf
+      || fn2 (l, -0xf) != 0xf
+      || fn2 (l, -0x2468acf) != 0x2468acf
+      || fn2 (l, -0x91a2b3c) != 0x91a2b3c)
+    __builtin_abort ();
+  if (fn3 (l, 1) != -1
+      || fn3 (l, -1) != 1
+      || fn3 (l, 0) != 0
+      || fn3 (l, 0x186A0) != -0x186A0
+      || fn3 (l, 0xb3c2) != -0xb3c2
+      || fn3 (l, 0xb3) != -0xb3
+      || fn3 (l, 0xacf) != -0xacf
+      || fn3 (l, 0xf) != -0xf
+      || fn3 (l, 0x2468acf) != -0x2468acf
+      || fn3 (l, 0x91a2b3c) != -0x91a2b3c
+      || fn3 (l, -0x186A0) != 0x186A0
+      || fn3 (l, -0xb3c2) != 0xb3c2
+      || fn3 (l, -0xb3) != 0xb3
+      || fn3 (l, -0xacf) != 0xacf
+      || fn3 (l, -0xf) != 0xf
+      || fn3 (l, -0x2468acf) != 0x2468acf
+      || fn3 (l, -0x91a2b3c) != 0x91a2b3c)
+    __builtin_abort ();
+
+  return 0;
+}
+
+/* { dg-final { scan-tree-dump-not " - " "forwprop1" } } */
+/* { dg-final { cleanup-tree-dump "forwprop1" } } */
diff --git gcc/tree-ssa-forwprop.c gcc/tree-ssa-forwprop.c
index 0284301..c59f1d7 100644
--- gcc/tree-ssa-forwprop.c
+++ gcc/tree-ssa-forwprop.c
@@ -2534,13 +2534,14 @@ associate_plusminus (gimple_stmt_iterator *gsi)
 	(CST +- A) +- CST  ->  CST +- A
 	(A +- CST) +- CST  ->  A +- CST
 	~A + A             ->  -1
-	~A + 1             ->  -A 
+	~A + 1             ->  -A
 	A - (A +- B)       ->  -+ B
 	A +- (B +- A)      ->  +- B
 	CST +- (CST +- A)  ->  CST +- A
 	CST +- (A +- CST)  ->  CST +- A
 	A + ~A             ->  -1
 	(T)(P + A) - (T)P  -> (T)A
+	(T)P - (T)(P + A)  -> (T)-A
 
      via commutating the addition and contracting operations to zero
      by reassociation.  */
@@ -2799,6 +2800,76 @@ associate_plusminus (gimple_stmt_iterator *gsi)
 		  gimple_set_modified (stmt, true);
 		}
 	    }
+	  else if (code == MINUS_EXPR
+		   && CONVERT_EXPR_CODE_P (def_code)
+		   && TREE_CODE (gimple_assign_rhs1 (def_stmt)) == SSA_NAME
+		   && TREE_CODE (rhs1) == SSA_NAME)
+	    {
+	      /* (T)P - (T)(P + A) -> (T)-A.  */
+	      gimple def_stmt2 = SSA_NAME_DEF_STMT (rhs1);
+	      if (is_gimple_assign (def_stmt2)
+		  && can_propagate_from (def_stmt2)
+		  && CONVERT_EXPR_CODE_P (gimple_assign_rhs_code (def_stmt2))
+		  && TREE_CODE (gimple_assign_rhs1 (def_stmt2)) == SSA_NAME)
+		{
+		  /* Now we have (T)P - (T)X.  */
+		  tree p = gimple_assign_rhs1 (def_stmt2);
+		  def_stmt2 = SSA_NAME_DEF_STMT (gimple_assign_rhs1 (def_stmt));
+		  if (is_gimple_assign (def_stmt2)
+		      && can_propagate_from (def_stmt2)
+		      && (gimple_assign_rhs_code (def_stmt2) == POINTER_PLUS_EXPR
+			  || gimple_assign_rhs_code (def_stmt2) == PLUS_EXPR)
+		      && gimple_assign_rhs1 (def_stmt2) == p)
+		    {
+		      /* And finally (T)P - (T)(P + A).  */
+		      tree a = gimple_assign_rhs2 (def_stmt2);
+		      if (TYPE_PRECISION (TREE_TYPE (rhs1))
+			  <= TYPE_PRECISION (TREE_TYPE (a))
+			  /* For integer types, if A has a smaller type
+			     than T the result depends on the possible
+			     overflow in P + A.
+			     E.g. T=size_t, A=(unsigned)429497295, P>0.
+			     However, if an overflow in P + A would cause
+			     undefined behavior, we can assume that there
+			     is no overflow.  */
+			  || (INTEGRAL_TYPE_P (TREE_TYPE (p))
+			      && TYPE_OVERFLOW_UNDEFINED (TREE_TYPE (p)))
+			  /* For pointer types, if the conversion of A to the
+			     final type requires a sign- or zero-extension,
+			     then we have to punt - it is not defined which
+			     one is correct.  */
+			  || (POINTER_TYPE_P (TREE_TYPE (p))
+			      && TREE_CODE (a) == INTEGER_CST
+			      && tree_int_cst_sign_bit (a) == 0))
+			{
+			  if (issue_strict_overflow_warning
+			      (WARN_STRICT_OVERFLOW_MISC)
+			      && TYPE_PRECISION (TREE_TYPE (rhs1))
+				 > TYPE_PRECISION (TREE_TYPE (a))
+			      && INTEGRAL_TYPE_P (TREE_TYPE (p)))
+			    warning_at (gimple_location (stmt),
+					OPT_Wstrict_overflow,
+					"assuming signed overflow does not "
+					"occur when assuming that "
+					"(T)P - (T)(P + A) is always (T)-A");
+			  if (!useless_type_conversion_p (TREE_TYPE (rhs1),
+							  TREE_TYPE (a)))
+			    {
+			      a = fold_convert (TREE_TYPE (rhs1), a);
+			      a = force_gimple_operand_gsi (gsi, a, true,
+							    NULL_TREE, true,
+							    GSI_SAME_STMT);
+			    }
+			  rhs1 = a;
+			  rhs2 = NULL_TREE;
+			  gimple_assign_set_rhs_with_ops (gsi, NEGATE_EXPR,
+							  rhs1, rhs2);
+			  gcc_assert (gsi_stmt (*gsi) == stmt);
+			  gimple_set_modified (stmt, true);
+			}
+		    }
+		}
+	    }
 	}
     }
 
	Marek


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