public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] Fix PR tree-optimization/58137
@ 2013-08-20 11:21 Bernd Edlinger
  2013-08-30 10:39 ` Richard Biener
  0 siblings, 1 reply; 7+ messages in thread
From: Bernd Edlinger @ 2013-08-20 11:21 UTC (permalink / raw)
  To: gcc-patches

[-- Attachment #1: Type: text/plain, Size: 971 bytes --]

This patch fixes a bug in the vectorized pointer arithmetic in the forwprop
optimization pass.

Although it seems to be impossible to create a vector of pointers with the
__attribute__((vector_size(x))) syntax, the vector loop optimization together
with the loop unrolling can create code which adds a vector of ptroff_t
repeatedly to a vector of pointers.

The code in tree-ssa-forwprop.c handles program transformations of the
form (PTR +- CST1) +- CST2 => PTR +- (CST1+-CST2) where PTR is
a vector of pointers and CST1 and CST2 are vectors of ptroff_t, without the
fix the result type of CST1+-CST2 was vector of pointer, which causes the
ICE in tree-cfg.c, which sees an undefined pointer + pointer operation.

Additionally the check in tree-cfg.c allows expressions of the form CST - PTR,
which is clearly wrong. That should be fixed too.

The patch was bootstrapped and regression tested on i686-pc-linux-gnu.

Regards
Bernd Edlinger 		 	   		  

[-- Attachment #2: changelog-forwprop.txt --]
[-- Type: text/plain, Size: 496 bytes --]

2013-08-20  Bernd Edlinger  <bernd.edlinger@hotmail.de>

	PR tree-optimization/58137
	Fixed vectorized pointer arithmentic in constant folding at forwprop.
	Improved checks in verify_gimple_in_cfg: MINUS_EXPR with pointer vectors
	is not commutative.

	* tree-cfg.c (verify_gimple_assign_binary): Only PLUS_EXPR is commutative.
	* tree-ssa-forwprop.c (fold_const_type): New function.
	(associate_plusminus): Fold constant pointer vector types correctly.

	* gcc.target/i386/pr58137.c: New test.


[-- Attachment #3: patch-forwprop.diff --]
[-- Type: application/octet-stream, Size: 3845 bytes --]

--- gcc/tree-cfg.c.jj	2013-08-05 22:16:05.000000000 +0200
+++ gcc/tree-cfg.c	2013-08-13 00:14:47.000000000 +0200
@@ -3588,7 +3588,7 @@ verify_gimple_assign_binary (gimple stmt
 	    rhs2_type = TREE_TYPE (rhs2_type);
 	    /* PLUS_EXPR is commutative, so we might end up canonicalizing
 	       the pointer to 2nd place.  */
-	    if (POINTER_TYPE_P (rhs2_type))
+	    if (POINTER_TYPE_P (rhs2_type) && rhs_code == PLUS_EXPR)
 	      {
 		tree tem = rhs1_type;
 		rhs1_type = rhs2_type;
--- gcc/tree-ssa-forwprop.c.jj	2013-08-05 22:16:05.000000000 +0200
+++ gcc/tree-ssa-forwprop.c	2013-08-13 01:19:48.000000000 +0200
@@ -2442,6 +2442,26 @@ simplify_rotate (gimple_stmt_iterator *g
   return true;
 }
 
+/* Determine the correct result type for constant folding.
+   VARIBLE, CONSTANT1 and CONSTANT2 are types of the expressions
+   in the statement.  Usually the result type is VARIABLE.
+   But if VARIABLE is a vector of pointers and CONSTANT1 and
+   CONSTANT2 are vectors of ptrofftype, the result is CONSTANT1.  */
+
+static tree
+fold_const_type (tree variable, tree constant1, tree constant2)
+{
+  if (TREE_CODE (variable) == VECTOR_TYPE
+      && POINTER_TYPE_P (TREE_TYPE (variable))
+      && TREE_CODE (constant1) == VECTOR_TYPE
+      && ptrofftype_p (TREE_TYPE (constant1))
+      && TREE_CODE (constant2) == VECTOR_TYPE
+      && ptrofftype_p (TREE_TYPE (constant2)))
+    return constant1;
+  else
+    return variable;
+}
+
 /* Perform re-associations of the plus or minus statement STMT that are
    always permitted.  Returns true if the CFG was changed.  */
 
@@ -2564,7 +2584,8 @@ associate_plusminus (gimple_stmt_iterato
 		       && CONSTANT_CLASS_P (def_rhs1))
 		{
 		  /* (CST +- A) +- CST -> CST +- A.  */
-		  tree cst = fold_binary (code, TREE_TYPE (rhs1),
+		  tree cst = fold_binary (code, fold_const_type (TREE_TYPE (rhs1),
+					  TREE_TYPE (def_rhs1), TREE_TYPE (rhs2)),
 					  def_rhs1, rhs2);
 		  if (cst && !TREE_OVERFLOW (cst))
 		    {
@@ -2583,7 +2604,8 @@ associate_plusminus (gimple_stmt_iterato
 		  /* (A +- CST) +- CST -> A +- CST.  */
 		  enum tree_code mix = (code == def_code)
 				       ? PLUS_EXPR : MINUS_EXPR;
-		  tree cst = fold_binary (mix, TREE_TYPE (rhs1),
+		  tree cst = fold_binary (mix, fold_const_type (TREE_TYPE (rhs1),
+					  TREE_TYPE (def_rhs2), TREE_TYPE (rhs2)),
 					  def_rhs2, rhs2);
 		  if (cst && !TREE_OVERFLOW (cst))
 		    {
@@ -2667,7 +2689,8 @@ associate_plusminus (gimple_stmt_iterato
 		       && CONSTANT_CLASS_P (def_rhs1))
 		{
 		  /* CST +- (CST +- A) -> CST +- A.  */
-		  tree cst = fold_binary (code, TREE_TYPE (rhs2),
+		  tree cst = fold_binary (code, fold_const_type (TREE_TYPE (rhs2),
+					  TREE_TYPE (rhs1), TREE_TYPE (def_rhs1)),
 					  rhs1, def_rhs1);
 		  if (cst && !TREE_OVERFLOW (cst))
 		    {
@@ -2686,7 +2709,8 @@ associate_plusminus (gimple_stmt_iterato
 		  /* CST +- (A +- CST) -> CST +- A.  */
 		  tree cst = fold_binary (def_code == code
 					  ? PLUS_EXPR : MINUS_EXPR,
-					  TREE_TYPE (rhs2),
+					  fold_const_type (TREE_TYPE (rhs2),
+					  TREE_TYPE (rhs1), TREE_TYPE (def_rhs2)),
 					  rhs1, def_rhs2);
 		  if (cst && !TREE_OVERFLOW (cst))
 		    {
--- /dev/null	2013-08-04 16:05:15.327296207 +0200
+++ gcc/testsuite/gcc.target/i386/pr58137.c	2013-08-13 17:11:25.000000000 +0200
@@ -0,0 +1,33 @@
+/* { dg-do compile } */
+/* { dg-options "-O3 -mavx2" } */
+
+typedef unsigned int U32;
+
+struct sv {
+  void* sv_any;
+  U32 sv_refcnt;
+  U32 sv_flags;
+};
+typedef struct sv SV;
+
+struct xrv {
+  SV * xrv_rv;
+};
+typedef struct xrv XRV;
+
+extern XRV * PL_xrv_root;
+
+void
+more_xrv (void)
+{
+  register XRV* xrv;
+  register XRV* xrvend;
+  xrv = PL_xrv_root;
+  xrvend = &xrv[200 / sizeof (XRV) - 1];
+  while (xrv < xrvend)
+  {
+    xrv->xrv_rv = (SV*)(xrv + 1);
+    xrv++;
+  }
+  xrv->xrv_rv = 0;
+}

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] Fix PR tree-optimization/58137
  2013-08-20 11:21 [PATCH] Fix PR tree-optimization/58137 Bernd Edlinger
@ 2013-08-30 10:39 ` Richard Biener
  2013-09-02 18:53   ` Bernd Edlinger
  0 siblings, 1 reply; 7+ messages in thread
From: Richard Biener @ 2013-08-30 10:39 UTC (permalink / raw)
  To: Bernd Edlinger; +Cc: gcc-patches

On Tue, Aug 20, 2013 at 1:01 PM, Bernd Edlinger
<bernd.edlinger@hotmail.de> wrote:
> This patch fixes a bug in the vectorized pointer arithmetic in the forwprop
> optimization pass.
>
> Although it seems to be impossible to create a vector of pointers with the
> __attribute__((vector_size(x))) syntax, the vector loop optimization together
> with the loop unrolling can create code which adds a vector of ptroff_t
> repeatedly to a vector of pointers.
>
> The code in tree-ssa-forwprop.c handles program transformations of the
> form (PTR +- CST1) +- CST2 => PTR +- (CST1+-CST2) where PTR is
> a vector of pointers and CST1 and CST2 are vectors of ptroff_t, without the
> fix the result type of CST1+-CST2 was vector of pointer, which causes the
> ICE in tree-cfg.c, which sees an undefined pointer + pointer operation.
>
> Additionally the check in tree-cfg.c allows expressions of the form CST - PTR,
> which is clearly wrong. That should be fixed too.
>
> The patch was bootstrapped and regression tested on i686-pc-linux-gnu.

It seems to me that the forwprop code does not handle the fact that we
are permissive as to using PLUS_EXPR instead of POINTER_PLUS_EXPR
for vector pointer - offset additions.  So instead of doing this dance the
better (and more easily backportable) fix is to simply disable the transform
on pointer valued PLUS_EXPR.  Like with

  if (POINTER_TYPE_P (TREE_TYPE (gimple_assign_lhs (stmt))))
    return false;

at the beginning of the function.

The real fix is of course to make vector pointer operations properly
use POINTER_PLUS_EXPR ...

Richard.



> Regards
> Bernd Edlinger

^ permalink raw reply	[flat|nested] 7+ messages in thread

* RE: [PATCH] Fix PR tree-optimization/58137
  2013-08-30 10:39 ` Richard Biener
@ 2013-09-02 18:53   ` Bernd Edlinger
  2013-09-03  8:38     ` Richard Biener
  0 siblings, 1 reply; 7+ messages in thread
From: Bernd Edlinger @ 2013-09-02 18:53 UTC (permalink / raw)
  To: Richard Biener; +Cc: gcc-patches

On Fri, 30 Aug 2013 12:34:51 Richard Biener wrote:
> On Tue, Aug 20, 2013 at 1:01 PM, Bernd Edlinger
> <bernd.edlinger@hotmail.de> wrote:
>> This patch fixes a bug in the vectorized pointer arithmetic in the forwprop
>> optimization pass.
>>
>> Although it seems to be impossible to create a vector of pointers with the
>> __attribute__((vector_size(x))) syntax, the vector loop optimization together
>> with the loop unrolling can create code which adds a vector of ptroff_t
>> repeatedly to a vector of pointers.
>>
>> The code in tree-ssa-forwprop.c handles program transformations of the
>> form (PTR +- CST1) +- CST2 => PTR +- (CST1+-CST2) where PTR is
>> a vector of pointers and CST1 and CST2 are vectors of ptroff_t, without the
>> fix the result type of CST1+-CST2 was vector of pointer, which causes the
>> ICE in tree-cfg.c, which sees an undefined pointer + pointer operation.
>>
>> Additionally the check in tree-cfg.c allows expressions of the form CST - PTR,
>> which is clearly wrong. That should be fixed too.
>>
>> The patch was bootstrapped and regression tested on i686-pc-linux-gnu.
>
> It seems to me that the forwprop code does not handle the fact that we
> are permissive as to using PLUS_EXPR instead of POINTER_PLUS_EXPR
> for vector pointer - offset additions. So instead of doing this dance the
> better (and more easily backportable) fix is to simply disable the transform
> on pointer valued PLUS_EXPR. Like with
>
> if (POINTER_TYPE_P (TREE_TYPE (gimple_assign_lhs (stmt))))
> return false;
>
> at the beginning of the function.
>

the condition would have to be:

  if (TREE_CODE (TREE_TYPE (gimple_assign_lhs (stmt))) == VECTOR_TYPE
      && POINTER_TYPE_P (TREE_TYPE (TREE_TYPE (gimple_assign_lhs (stmt)))))
    return false;

I tried this, and it fixes the ICE. However the generated code was still vectorized but
much less efficient and used more registers.

Fortunately there will be no need to backport this, since this bug does not happen in
gcc 4.8.2 I checked that. I believe it was introduced with the checkin r200059 by
Marc Glisse where associate_plusminus was enhanced to handle vector values.
Before that only TREE_CODE (rhs2) == INTEGER_CST was handled.

Frankly I would prefer the initial version of the patch, because the code is more
efficient this way. The vector data is folded correctly, only the data type was wrong
and triggered the ICE in tree-cfg.c.

Please advise.

Thanks
Bernd.

> The real fix is of course to make vector pointer operations properly
> use POINTER_PLUS_EXPR ...
>
> Richard.
>
>
>
>> Regards
>> Bernd Edlinger 		 	   		  

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] Fix PR tree-optimization/58137
  2013-09-02 18:53   ` Bernd Edlinger
@ 2013-09-03  8:38     ` Richard Biener
  2013-09-05  9:43       ` Bernd Edlinger
  0 siblings, 1 reply; 7+ messages in thread
From: Richard Biener @ 2013-09-03  8:38 UTC (permalink / raw)
  To: Bernd Edlinger; +Cc: gcc-patches

On Mon, Sep 2, 2013 at 8:53 PM, Bernd Edlinger
<bernd.edlinger@hotmail.de> wrote:
> On Fri, 30 Aug 2013 12:34:51 Richard Biener wrote:
>> On Tue, Aug 20, 2013 at 1:01 PM, Bernd Edlinger
>> <bernd.edlinger@hotmail.de> wrote:
>>> This patch fixes a bug in the vectorized pointer arithmetic in the forwprop
>>> optimization pass.
>>>
>>> Although it seems to be impossible to create a vector of pointers with the
>>> __attribute__((vector_size(x))) syntax, the vector loop optimization together
>>> with the loop unrolling can create code which adds a vector of ptroff_t
>>> repeatedly to a vector of pointers.
>>>
>>> The code in tree-ssa-forwprop.c handles program transformations of the
>>> form (PTR +- CST1) +- CST2 => PTR +- (CST1+-CST2) where PTR is
>>> a vector of pointers and CST1 and CST2 are vectors of ptroff_t, without the
>>> fix the result type of CST1+-CST2 was vector of pointer, which causes the
>>> ICE in tree-cfg.c, which sees an undefined pointer + pointer operation.
>>>
>>> Additionally the check in tree-cfg.c allows expressions of the form CST - PTR,
>>> which is clearly wrong. That should be fixed too.
>>>
>>> The patch was bootstrapped and regression tested on i686-pc-linux-gnu.
>>
>> It seems to me that the forwprop code does not handle the fact that we
>> are permissive as to using PLUS_EXPR instead of POINTER_PLUS_EXPR
>> for vector pointer - offset additions. So instead of doing this dance the
>> better (and more easily backportable) fix is to simply disable the transform
>> on pointer valued PLUS_EXPR. Like with
>>
>> if (POINTER_TYPE_P (TREE_TYPE (gimple_assign_lhs (stmt))))
>> return false;
>>
>> at the beginning of the function.
>>
>
> the condition would have to be:
>
>   if (TREE_CODE (TREE_TYPE (gimple_assign_lhs (stmt))) == VECTOR_TYPE
>       && POINTER_TYPE_P (TREE_TYPE (TREE_TYPE (gimple_assign_lhs (stmt)))))
>     return false;
>
> I tried this, and it fixes the ICE. However the generated code was still vectorized but
> much less efficient and used more registers.
>
> Fortunately there will be no need to backport this, since this bug does not happen in
> gcc 4.8.2 I checked that. I believe it was introduced with the checkin r200059 by
> Marc Glisse where associate_plusminus was enhanced to handle vector values.
> Before that only TREE_CODE (rhs2) == INTEGER_CST was handled.
>
> Frankly I would prefer the initial version of the patch, because the code is more
> efficient this way. The vector data is folded correctly, only the data type was wrong
> and triggered the ICE in tree-cfg.c.
>
> Please advise.

I'd rather go with the simple fix as the issue in forwprop is at least
latent.  We can
improve on the code-gen as followup where I believe handling of
POINTER_PLUS_EXPR
would need to be added (that we avoid POINTER_PLUS_EXPR for vectors is a bug).
That can be done in a way to cover the vector case properly.  Or
finally properly
use POINTER_PLUS_EXPR for vectors or make the vectorizer not use pointer
types but a corresponding unsigned integer type for them (that would also fix
the original bug of course).  Like with (untested)

Index: gcc/tree-vect-stmts.c
===================================================================
--- gcc/tree-vect-stmts.c       (revision 202196)
+++ gcc/tree-vect-stmts.c       (working copy)
@@ -6179,8 +6179,7 @@ get_vectype_for_scalar_type_and_size (tr
      corresponding to that mode.  The theory is that any use that
      would cause problems with this will disable vectorization anyway.  */
   else if (!SCALAR_FLOAT_TYPE_P (scalar_type)
-          && !INTEGRAL_TYPE_P (scalar_type)
-          && !POINTER_TYPE_P (scalar_type))
+          && !INTEGRAL_TYPE_P (scalar_type))
     scalar_type = lang_hooks.types.type_for_mode (inner_mode, 1);

   /* We can't build a vector type of elements with alignment bigger than

actually that would be my preference here ...

Thanks,
Richard.

> Thanks
> Bernd.
>
>> The real fix is of course to make vector pointer operations properly
>> use POINTER_PLUS_EXPR ...
>>
>> Richard.
>>
>>
>>
>>> Regards
>>> Bernd Edlinger

^ permalink raw reply	[flat|nested] 7+ messages in thread

* RE: [PATCH] Fix PR tree-optimization/58137
  2013-09-03  8:38     ` Richard Biener
@ 2013-09-05  9:43       ` Bernd Edlinger
  2013-09-05 10:25         ` Richard Biener
  0 siblings, 1 reply; 7+ messages in thread
From: Bernd Edlinger @ 2013-09-05  9:43 UTC (permalink / raw)
  To: Richard Biener; +Cc: gcc-patches

[-- Attachment #1: Type: text/plain, Size: 1966 bytes --]

On Tue, 3 Sep 2013 10:38:33, Richard Biener wrote:
> I'd rather go with the simple fix as the issue in forwprop is at least
> latent. We can
> improve on the code-gen as followup where I believe handling of
> POINTER_PLUS_EXPR
> would need to be added (that we avoid POINTER_PLUS_EXPR for vectors is a bug).
> That can be done in a way to cover the vector case properly. Or
> finally properly
> use POINTER_PLUS_EXPR for vectors or make the vectorizer not use pointer
> types but a corresponding unsigned integer type for them (that would also fix
> the original bug of course). Like with (untested)
>
> Index: gcc/tree-vect-stmts.c
> ===================================================================
> --- gcc/tree-vect-stmts.c (revision 202196)
> +++ gcc/tree-vect-stmts.c (working copy)
> @@ -6179,8 +6179,7 @@ get_vectype_for_scalar_type_and_size (tr
> corresponding to that mode. The theory is that any use that
> would cause problems with this will disable vectorization anyway. */
> else if (!SCALAR_FLOAT_TYPE_P (scalar_type)
> - && !INTEGRAL_TYPE_P (scalar_type)
> - && !POINTER_TYPE_P (scalar_type))
> + && !INTEGRAL_TYPE_P (scalar_type))
> scalar_type = lang_hooks.types.type_for_mode (inner_mode, 1);
>
> /* We can't build a vector type of elements with alignment bigger than
>
> actually that would be my preference here ...

this would cause an ICE in test case 20000629-1.c...

So removing the pointer of vectors is not an option.

>>> The real fix is of course to make vector pointer operations properly
>>> use POINTER_PLUS_EXPR ...

Okay I can do what you want, and use POINTER_PLUS_EXPR for
vectors of pointers, and do the constant folding in assocate_pointerplus.
This way we get exactly the same code as before. It may be even possible
that this constant folding can improve something with scalars.

Bootstrapped and regression tested on x86_64-unknown-linux-gnu.

OK for trunk?

Bernd. 		 	   		  

[-- Attachment #2: changelog-forwprop.txt --]
[-- Type: text/plain, Size: 622 bytes --]

2013-09-05  Bernd Edlinger  <bernd.edlinger@hotmail.de>

	PR tree-optimization/58137
	Use POINTER_PLUS_EXPR for vector of pointer additions.
	* tree-cfg.c (verify_gimple_assign_binary)
	<PLUS_EXPR, MINUS_EXPR>: Remove vector of pointer handling.
	<POINTER_PLUS_EXPR>: Add vector of pointer type check.
	* tree.c (build2_stat) <POINTER_PLUS_EXPR>: Allow vector of pointer.
	* tree-vect-stmts.c (vectorizable_operation): Do not replace
	POINTER_PLUS_EXPR with PLUS_EXPR.
	* tree-vect-loop.c (get_initial_def_for_induction): Use
	POINTER_PLUS_EXPR for vector of pointer.

testsuite:

	* gcc.target/i386/pr58137.c: New test.


[-- Attachment #3: patch-forwprop.diff --]
[-- Type: application/octet-stream, Size: 6407 bytes --]

--- gcc/tree-cfg.c	2013-08-05 22:16:05.000000000 +0200
+++ gcc/tree-cfg.c	2013-09-04 20:01:15.000000000 +0200
@@ -3571,31 +3571,6 @@ verify_gimple_assign_binary (gimple stmt
     case PLUS_EXPR:
     case MINUS_EXPR:
       {
-	/* We use regular PLUS_EXPR and MINUS_EXPR for vectors.
-	   ???  This just makes the checker happy and may not be what is
-	   intended.  */
-	if (TREE_CODE (lhs_type) == VECTOR_TYPE
-	    && POINTER_TYPE_P (TREE_TYPE (lhs_type)))
-	  {
-	    if (TREE_CODE (rhs1_type) != VECTOR_TYPE
-		|| TREE_CODE (rhs2_type) != VECTOR_TYPE)
-	      {
-		error ("invalid non-vector operands to vector valued plus");
-		return true;
-	      }
-	    lhs_type = TREE_TYPE (lhs_type);
-	    rhs1_type = TREE_TYPE (rhs1_type);
-	    rhs2_type = TREE_TYPE (rhs2_type);
-	    /* PLUS_EXPR is commutative, so we might end up canonicalizing
-	       the pointer to 2nd place.  */
-	    if (POINTER_TYPE_P (rhs2_type))
-	      {
-		tree tem = rhs1_type;
-		rhs1_type = rhs2_type;
-		rhs2_type = tem;
-	      }
-	    goto do_pointer_plus_expr_check;
-	  }
 	if (POINTER_TYPE_P (lhs_type)
 	    || POINTER_TYPE_P (rhs1_type)
 	    || POINTER_TYPE_P (rhs2_type))
@@ -3610,7 +3585,18 @@ verify_gimple_assign_binary (gimple stmt
 
     case POINTER_PLUS_EXPR:
       {
-do_pointer_plus_expr_check:
+	if (TREE_CODE (lhs_type) == VECTOR_TYPE)
+	  {
+	    if (TREE_CODE (rhs1_type) != VECTOR_TYPE
+		|| TREE_CODE (rhs2_type) != VECTOR_TYPE)
+	      {
+		error ("invalid non-vector operands to vector valued pointer plus");
+		return true;
+	      }
+	    lhs_type = TREE_TYPE (lhs_type);
+	    rhs1_type = TREE_TYPE (rhs1_type);
+	    rhs2_type = TREE_TYPE (rhs2_type);
+	  }
 	if (!POINTER_TYPE_P (rhs1_type)
 	    || !useless_type_conversion_p (lhs_type, rhs1_type)
 	    || !ptrofftype_p (rhs2_type))
--- gcc/tree.c	2013-08-24 00:16:08.000000000 +0200
+++ gcc/tree.c	2013-09-04 20:01:08.000000000 +0200
@@ -4092,8 +4092,14 @@ build2_stat (enum tree_code code, tree t
 		&& TREE_CODE (arg1) == INTEGER_CST);
 
   if (code == POINTER_PLUS_EXPR && arg0 && arg1 && tt)
-    gcc_assert (POINTER_TYPE_P (tt) && POINTER_TYPE_P (TREE_TYPE (arg0))
-		&& ptrofftype_p (TREE_TYPE (arg1)));
+    gcc_assert ((POINTER_TYPE_P (tt) && POINTER_TYPE_P (TREE_TYPE (arg0))
+		 && ptrofftype_p (TREE_TYPE (arg1)))
+		|| (TREE_CODE (tt) == VECTOR_TYPE
+		    && TREE_CODE (TREE_TYPE (arg0)) == VECTOR_TYPE
+		    && TREE_CODE (TREE_TYPE (arg1)) == VECTOR_TYPE
+		    && POINTER_TYPE_P (TREE_TYPE (tt))
+		    && POINTER_TYPE_P (TREE_TYPE (TREE_TYPE (arg0)))
+		    && ptrofftype_p (TREE_TYPE (TREE_TYPE (arg1)))));
 
   t = make_node_stat (code PASS_MEM_STAT);
   TREE_TYPE (t) = tt;
--- gcc/tree-vect-stmts.c	2013-06-27 17:35:57.000000000 +0200
+++ gcc/tree-vect-stmts.c	2013-09-04 20:01:44.000000000 +0200
@@ -3470,11 +3470,6 @@ vectorizable_operation (gimple stmt, gim
 
   code = gimple_assign_rhs_code (stmt);
 
-  /* For pointer addition, we should use the normal plus for
-     the vector addition.  */
-  if (code == POINTER_PLUS_EXPR)
-    code = PLUS_EXPR;
-
   /* Support only unary or binary operations.  */
   op_type = TREE_CODE_LENGTH (code);
   if (op_type != unary_op && op_type != binary_op && op_type != ternary_op)
--- gcc/tree-vect-loop.c	2013-07-02 13:54:09.000000000 +0200
+++ gcc/tree-vect-loop.c	2013-09-04 20:02:38.000000000 +0200
@@ -3323,8 +3323,9 @@ get_initial_def_for_induction (gimple iv
   induc_def = PHI_RESULT (induction_phi);
 
   /* Create the iv update inside the loop  */
-  new_stmt = gimple_build_assign_with_ops (PLUS_EXPR, vec_dest,
-					   induc_def, vec_step);
+  new_stmt = gimple_build_assign_with_ops (POINTER_TYPE_P (scalar_type)
+					   ? POINTER_PLUS_EXPR : PLUS_EXPR,
+					   vec_dest, induc_def, vec_step);
   vec_def = make_ssa_name (vec_dest, new_stmt);
   gimple_assign_set_lhs (new_stmt, vec_def);
   gsi_insert_before (&si, new_stmt, GSI_SAME_STMT);
--- gcc/tree-ssa-forwprop.c	2013-08-05 22:16:05.000000000 +0200
+++ gcc/tree-ssa-forwprop.c	2013-09-04 23:13:43.000000000 +0200
@@ -2739,6 +2739,44 @@ associate_pointerplus (gimple_stmt_itera
   gimple def_stmt;
   tree ptr, rhs, algn;
 
+  ptr = gimple_assign_rhs1 (stmt);
+  rhs = gimple_assign_rhs2 (stmt);
+
+  /* Pattern match
+       tem = ptr p+ cst1;
+       ... = tem p+ cst2;
+     and produce the simpler
+       ... = ptr p+ cst; */
+  if (TREE_CODE (ptr) == SSA_NAME)
+    {
+      def_stmt = SSA_NAME_DEF_STMT (ptr);
+      if (is_gimple_assign (def_stmt) && can_propagate_from (def_stmt))
+	{
+	  enum tree_code def_code = gimple_assign_rhs_code (def_stmt);
+	  if (def_code == POINTER_PLUS_EXPR)
+	    {
+	      tree def_ptr = gimple_assign_rhs1 (def_stmt);
+	      tree def_rhs = gimple_assign_rhs2 (def_stmt);
+	      if (CONSTANT_CLASS_P (rhs)
+		  && CONSTANT_CLASS_P (def_rhs))
+		{
+		  /* (A p+ CST) p+ CST -> A p+ CST.  */
+		  tree cst = fold_binary (PLUS_EXPR, TREE_TYPE (rhs),
+					  def_rhs, rhs);
+		  if (cst && !TREE_OVERFLOW (cst))
+		    {
+		      gimple_assign_set_rhs_code (stmt, def_code);
+		      gimple_assign_set_rhs1 (stmt, def_ptr);
+		      gimple_assign_set_rhs2 (stmt, cst);
+		      fold_stmt_inplace (gsi);
+		      update_stmt (stmt);
+		      return true;
+		    }
+		}
+	    }
+        }
+    }
+
   /* Pattern match
        tem = (sizetype) ptr;
        tem = tem & algn;
@@ -2746,8 +2784,6 @@ associate_pointerplus (gimple_stmt_itera
        ... = ptr p+ tem;
      and produce the simpler and easier to analyze with respect to alignment
        ... = ptr & ~algn;  */
-  ptr = gimple_assign_rhs1 (stmt);
-  rhs = gimple_assign_rhs2 (stmt);
   if (TREE_CODE (rhs) != SSA_NAME)
     return false;
   def_stmt = SSA_NAME_DEF_STMT (rhs);
--- gcc/testsuite/gcc.target/i386/pr58137.c	1970-01-01 01:00:00.000000000 +0100
+++ gcc/testsuite/gcc.target/i386/pr58137.c	2013-08-19 17:35:57.000000000 +0200
@@ -0,0 +1,34 @@
+/* PR tree-optimization/58137 */
+/* { dg-do compile } */
+/* { dg-options "-O3 -mavx2" } */
+
+typedef unsigned int U32;
+
+struct sv {
+  void* sv_any;
+  U32 sv_refcnt;
+  U32 sv_flags;
+};
+typedef struct sv SV;
+
+struct xrv {
+  SV * xrv_rv;
+};
+typedef struct xrv XRV;
+
+extern XRV * PL_xrv_root;
+
+void
+more_xrv (void)
+{
+  register XRV* xrv;
+  register XRV* xrvend;
+  xrv = PL_xrv_root;
+  xrvend = &xrv[200 / sizeof (XRV) - 1];
+  while (xrv < xrvend)
+  {
+    xrv->xrv_rv = (SV*)(xrv + 1);
+    xrv++;
+  }
+  xrv->xrv_rv = 0;
+}

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] Fix PR tree-optimization/58137
  2013-09-05  9:43       ` Bernd Edlinger
@ 2013-09-05 10:25         ` Richard Biener
  2013-09-05 10:52           ` Bernd Edlinger
  0 siblings, 1 reply; 7+ messages in thread
From: Richard Biener @ 2013-09-05 10:25 UTC (permalink / raw)
  To: Bernd Edlinger; +Cc: gcc-patches

On Thu, Sep 5, 2013 at 11:43 AM, Bernd Edlinger
<bernd.edlinger@hotmail.de> wrote:
> On Tue, 3 Sep 2013 10:38:33, Richard Biener wrote:
>> I'd rather go with the simple fix as the issue in forwprop is at least
>> latent. We can
>> improve on the code-gen as followup where I believe handling of
>> POINTER_PLUS_EXPR
>> would need to be added (that we avoid POINTER_PLUS_EXPR for vectors is a bug).
>> That can be done in a way to cover the vector case properly. Or
>> finally properly
>> use POINTER_PLUS_EXPR for vectors or make the vectorizer not use pointer
>> types but a corresponding unsigned integer type for them (that would also fix
>> the original bug of course). Like with (untested)
>>
>> Index: gcc/tree-vect-stmts.c
>> ===================================================================
>> --- gcc/tree-vect-stmts.c (revision 202196)
>> +++ gcc/tree-vect-stmts.c (working copy)
>> @@ -6179,8 +6179,7 @@ get_vectype_for_scalar_type_and_size (tr
>> corresponding to that mode. The theory is that any use that
>> would cause problems with this will disable vectorization anyway. */
>> else if (!SCALAR_FLOAT_TYPE_P (scalar_type)
>> - && !INTEGRAL_TYPE_P (scalar_type)
>> - && !POINTER_TYPE_P (scalar_type))
>> + && !INTEGRAL_TYPE_P (scalar_type))
>> scalar_type = lang_hooks.types.type_for_mode (inner_mode, 1);
>>
>> /* We can't build a vector type of elements with alignment bigger than
>>
>> actually that would be my preference here ...
>
> this would cause an ICE in test case 20000629-1.c...

Well, that's easily fixable.

> So removing the pointer of vectors is not an option.

Let me nevertheless try this.

Richard.

>>>> The real fix is of course to make vector pointer operations properly
>>>> use POINTER_PLUS_EXPR ...
>
> Okay I can do what you want, and use POINTER_PLUS_EXPR for
> vectors of pointers, and do the constant folding in assocate_pointerplus.
> This way we get exactly the same code as before. It may be even possible
> that this constant folding can improve something with scalars.
>
> Bootstrapped and regression tested on x86_64-unknown-linux-gnu.
>
> OK for trunk?
>
> Bernd.

^ permalink raw reply	[flat|nested] 7+ messages in thread

* RE: [PATCH] Fix PR tree-optimization/58137
  2013-09-05 10:25         ` Richard Biener
@ 2013-09-05 10:52           ` Bernd Edlinger
  0 siblings, 0 replies; 7+ messages in thread
From: Bernd Edlinger @ 2013-09-05 10:52 UTC (permalink / raw)
  To: Richard Biener; +Cc: gcc-patches

On Thu, 5 Sep 2013 12:25:08, Richard Biener wrote:
> On Thu, Sep 5, 2013 at 11:43 AM, Bernd Edlinger
> <bernd.edlinger@hotmail.de> wrote:
>>
>> this would cause an ICE in test case 20000629-1.c...
>
> Well, that's easily fixable.
>
>> So removing the pointer of vectors is not an option.
>
> Let me nevertheless try this.
>
> Richard.

Very well, then you continue on this PR and I'll come back
later with a follow up patch.

Bernd. 		 	   		  

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2013-09-05 10:52 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-08-20 11:21 [PATCH] Fix PR tree-optimization/58137 Bernd Edlinger
2013-08-30 10:39 ` Richard Biener
2013-09-02 18:53   ` Bernd Edlinger
2013-09-03  8:38     ` Richard Biener
2013-09-05  9:43       ` Bernd Edlinger
2013-09-05 10:25         ` Richard Biener
2013-09-05 10:52           ` Bernd Edlinger

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).