Re: [PATCH] Fix up reduction-1{1,2} testcases (PR middle-end/68221)

[Jakub Jelinek]

On Thu, Jan 21, 2016 at 07:17:59AM +0100, Thomas Schwinge wrote:
> Ping.

I'd prefer to keep the code as is, it is closer to what could result from
the user trying to do a similar thing, and thus has a better chance of
keeping being supported.

Jakub

Re: [PATCH] Fix up reduction-1{1,2} testcases (PR middle-end/68221)

[Thomas Schwinge]

Hi!

Ping.

On Mon, 11 Jan 2016 11:40:58 +0100, I wrote:
> Ping.
> 
> On Wed, 23 Dec 2015 12:03:48 +0100, I wrote:
> > Ping.
> > 
> > On Thu, 26 Nov 2015 14:31:56 +0100, I wrote:
> > > On Mon, 23 Nov 2015 12:13:07 +0100 (CET), Richard Biener 
> > >  wrote:
> > > > On Fri, 20 Nov 2015, Jakub Jelinek wrote:
> > > > > If C/C++ array section reductions have non-zero (positive) bias, it is
> > > > > implemented by declaring a smaller private array and subtracting the 
> > > > > bias
> > > > > from the start of the private array (because valid code may only 
> > > > > dereference
> > > > > elements from bias onwards).  But, this isn't something that is 
> > > > > kosher in
> > > > > C/C++ pointer arithmetics and the alias oracle seems to get upset on 
> > > > > that.
> > > > > So, the following patch fixes that by performing the subtraction on 
> > > > > integral
> > > > > type instead of p+ -bias.
> > > > 
> > > > So this still does use the biased pointer because you do not
> > > > re-write accesses (where you could have applied the biasing to
> > > > the indexes / offsets), right?  Thus the patch is merely obfuscation
> > > > for GCC rather than making it kosher for C/C++ (you still have a
> > > > pointer pointing outside of the private array object)?
> > > > 
> > > > I still hope to have a look where the alias oracle gets things
> > > > wrong (well, if so by accident at least).
> > > 
> > > I understand this ("have a look where the alias oracle gets things
> > > wrong") to have happened in Richi's trunk r230793,
> > > ?
> > > 
> > > I've tested that with the original POINTER_PLUS_EXPR code restored and
> > > with Richi's r230793 applied, for x86_64 GNU/Linux there is no change for
> > > the libgomp.c/reduction-11.c, libgomp.c/reduction-12.c,
> > > libgomp.c++/reduction-11.C, libgomp.c++/reduction-12.C test cases
> > > (already PASSed), but for 32-bit x86, they now PASS instead of FAILing.
> > > That is, I tested with the following (part of r230672) reverted:
> > > 
> > > > > --- gcc/omp-low.c.jj  2015-11-20 12:56:17.0 +0100
> > > > > +++ gcc/omp-low.c 2015-11-20 13:44:29.080374051 +0100
> > > > > @@ -,11 +,13 @@ lower_rec_input_clauses (tree clauses, g
> > > > >  
> > > > > if (!integer_zerop (bias))
> > > > >   {
> > > > > -   bias = fold_convert_loc (clause_loc, sizetype, bias);
> > > > > -   bias = fold_build1_loc (clause_loc, NEGATE_EXPR,
> > > > > -   sizetype, bias);
> > > > > -   x = fold_build2_loc (clause_loc, POINTER_PLUS_EXPR,
> > > > > -TREE_TYPE (x), x, bias);
> > > > > +   bias = fold_convert_loc (clause_loc, 
> > > > > pointer_sized_int_node,
> > > > > +bias);
> > > > > +   yb = fold_convert_loc (clause_loc, 
> > > > > pointer_sized_int_node,
> > > > > +  x);
> > > > > +   yb = fold_build2_loc (clause_loc, MINUS_EXPR,
> > > > > + pointer_sized_int_node, yb, 
> > > > > bias);
> > > > > +   x = fold_convert_loc (clause_loc, TREE_TYPE (x), yb);
> > > > > yb = create_tmp_var (ptype, name);
> > > > > gimplify_assign (yb, x, ilist);
> > > > > x = yb;
> > > 
> > > OK to commit the following to trunk?
> > > 
> > > commit 92b0eebfcbe914d3addeb97d4bb33f76a44dbe60
> > > Author: Thomas Schwinge 
> > > Date:   Thu Nov 26 14:21:13 2015 +0100
> > > 
> > > Restore original POINTER_PLUS_EXPR code
> > > 
> > >   PR middle-end/68221
> > >   gcc/
> > >   * omp-low.c (lower_rec_input_clauses): If C/C++ array reduction
> > >   has non-zero bias, use pointer plus of negated bias instead of
> > >   subtracting it in integer type.
> > > ---
> > >  gcc/omp-low.c |   12 +---
> > >  1 file changed, 5 insertions(+), 7 deletions(-)
> > > 
> > > diff --git gcc/omp-low.c gcc/omp-low.c
> > > index 0b44588..927d9d9 100644
> > > --- gcc/omp-low.c
> > > +++ gcc/omp-low.c
> > > @@ -4451,13 +4451,11 @@ lower_rec_input_clauses (tree clauses, gimple_seq 
> > > *ilist, gimple_seq *dlist,
> > >  
> > > if (!integer_zerop (bias))
> > >   {
> > > -   bias = fold_convert_loc (clause_loc, pointer_sized_int_node,
> > > -bias);
> > > -   yb = fold_convert_loc (clause_loc, pointer_sized_int_node,
> > > -  x);
> > > -   yb = fold_build2_loc (clause_loc, MINUS_EXPR,
> > > - pointer_sized_int_node, yb, bias);
> > > -   x = fold_convert_loc (clause_loc, TREE_TYPE (x), yb);
> > > +   bias = fold_convert_loc (clause_loc, sizetype, bias);
> > > +   bias = fold_build1_loc 

Re: [PATCH] Fix up reduction-1{1,2} testcases (PR middle-end/68221)

[Thomas Schwinge]

Hi!

Ping.

On Wed, 23 Dec 2015 12:03:48 +0100, I wrote:
> Ping.
> 
> On Thu, 26 Nov 2015 14:31:56 +0100, I wrote:
> > On Mon, 23 Nov 2015 12:13:07 +0100 (CET), Richard Biener 
> >  wrote:
> > > On Fri, 20 Nov 2015, Jakub Jelinek wrote:
> > > > If C/C++ array section reductions have non-zero (positive) bias, it is
> > > > implemented by declaring a smaller private array and subtracting the 
> > > > bias
> > > > from the start of the private array (because valid code may only 
> > > > dereference
> > > > elements from bias onwards).  But, this isn't something that is kosher 
> > > > in
> > > > C/C++ pointer arithmetics and the alias oracle seems to get upset on 
> > > > that.
> > > > So, the following patch fixes that by performing the subtraction on 
> > > > integral
> > > > type instead of p+ -bias.
> > > 
> > > So this still does use the biased pointer because you do not
> > > re-write accesses (where you could have applied the biasing to
> > > the indexes / offsets), right?  Thus the patch is merely obfuscation
> > > for GCC rather than making it kosher for C/C++ (you still have a
> > > pointer pointing outside of the private array object)?
> > > 
> > > I still hope to have a look where the alias oracle gets things
> > > wrong (well, if so by accident at least).
> > 
> > I understand this ("have a look where the alias oracle gets things
> > wrong") to have happened in Richi's trunk r230793,
> > ?
> > 
> > I've tested that with the original POINTER_PLUS_EXPR code restored and
> > with Richi's r230793 applied, for x86_64 GNU/Linux there is no change for
> > the libgomp.c/reduction-11.c, libgomp.c/reduction-12.c,
> > libgomp.c++/reduction-11.C, libgomp.c++/reduction-12.C test cases
> > (already PASSed), but for 32-bit x86, they now PASS instead of FAILing.
> > That is, I tested with the following (part of r230672) reverted:
> > 
> > > > --- gcc/omp-low.c.jj2015-11-20 12:56:17.0 +0100
> > > > +++ gcc/omp-low.c   2015-11-20 13:44:29.080374051 +0100
> > > > @@ -,11 +,13 @@ lower_rec_input_clauses (tree clauses, g
> > > >  
> > > >   if (!integer_zerop (bias))
> > > > {
> > > > - bias = fold_convert_loc (clause_loc, sizetype, bias);
> > > > - bias = fold_build1_loc (clause_loc, NEGATE_EXPR,
> > > > - sizetype, bias);
> > > > - x = fold_build2_loc (clause_loc, POINTER_PLUS_EXPR,
> > > > -  TREE_TYPE (x), x, bias);
> > > > + bias = fold_convert_loc (clause_loc, 
> > > > pointer_sized_int_node,
> > > > +  bias);
> > > > + yb = fold_convert_loc (clause_loc, 
> > > > pointer_sized_int_node,
> > > > +x);
> > > > + yb = fold_build2_loc (clause_loc, MINUS_EXPR,
> > > > +   pointer_sized_int_node, yb, 
> > > > bias);
> > > > + x = fold_convert_loc (clause_loc, TREE_TYPE (x), yb);
> > > >   yb = create_tmp_var (ptype, name);
> > > >   gimplify_assign (yb, x, ilist);
> > > >   x = yb;
> > 
> > OK to commit the following to trunk?
> > 
> > commit 92b0eebfcbe914d3addeb97d4bb33f76a44dbe60
> > Author: Thomas Schwinge 
> > Date:   Thu Nov 26 14:21:13 2015 +0100
> > 
> > Restore original POINTER_PLUS_EXPR code
> > 
> > PR middle-end/68221
> > gcc/
> > * omp-low.c (lower_rec_input_clauses): If C/C++ array reduction
> > has non-zero bias, use pointer plus of negated bias instead of
> > subtracting it in integer type.
> > ---
> >  gcc/omp-low.c |   12 +---
> >  1 file changed, 5 insertions(+), 7 deletions(-)
> > 
> > diff --git gcc/omp-low.c gcc/omp-low.c
> > index 0b44588..927d9d9 100644
> > --- gcc/omp-low.c
> > +++ gcc/omp-low.c
> > @@ -4451,13 +4451,11 @@ lower_rec_input_clauses (tree clauses, gimple_seq 
> > *ilist, gimple_seq *dlist,
> >  
> >   if (!integer_zerop (bias))
> > {
> > - bias = fold_convert_loc (clause_loc, pointer_sized_int_node,
> > -  bias);
> > - yb = fold_convert_loc (clause_loc, pointer_sized_int_node,
> > -x);
> > - yb = fold_build2_loc (clause_loc, MINUS_EXPR,
> > -   pointer_sized_int_node, yb, bias);
> > - x = fold_convert_loc (clause_loc, TREE_TYPE (x), yb);
> > + bias = fold_convert_loc (clause_loc, sizetype, bias);
> > + bias = fold_build1_loc (clause_loc, NEGATE_EXPR,
> > + sizetype, bias);
> > + x = fold_build2_loc (clause_loc, POINTER_PLUS_EXPR,
> > +  

Re: [PATCH] Fix up reduction-1{1,2} testcases (PR middle-end/68221)

[Thomas Schwinge]

Hi!

Ping.

On Thu, 26 Nov 2015 14:31:56 +0100, I wrote:
> On Mon, 23 Nov 2015 12:13:07 +0100 (CET), Richard Biener  
> wrote:
> > On Fri, 20 Nov 2015, Jakub Jelinek wrote:
> > > If C/C++ array section reductions have non-zero (positive) bias, it is
> > > implemented by declaring a smaller private array and subtracting the bias
> > > from the start of the private array (because valid code may only 
> > > dereference
> > > elements from bias onwards).  But, this isn't something that is kosher in
> > > C/C++ pointer arithmetics and the alias oracle seems to get upset on that.
> > > So, the following patch fixes that by performing the subtraction on 
> > > integral
> > > type instead of p+ -bias.
> > 
> > So this still does use the biased pointer because you do not
> > re-write accesses (where you could have applied the biasing to
> > the indexes / offsets), right?  Thus the patch is merely obfuscation
> > for GCC rather than making it kosher for C/C++ (you still have a
> > pointer pointing outside of the private array object)?
> > 
> > I still hope to have a look where the alias oracle gets things
> > wrong (well, if so by accident at least).
> 
> I understand this ("have a look where the alias oracle gets things
> wrong") to have happened in Richi's trunk r230793,
> ?
> 
> I've tested that with the original POINTER_PLUS_EXPR code restored and
> with Richi's r230793 applied, for x86_64 GNU/Linux there is no change for
> the libgomp.c/reduction-11.c, libgomp.c/reduction-12.c,
> libgomp.c++/reduction-11.C, libgomp.c++/reduction-12.C test cases
> (already PASSed), but for 32-bit x86, they now PASS instead of FAILing.
> That is, I tested with the following (part of r230672) reverted:
> 
> > > --- gcc/omp-low.c.jj  2015-11-20 12:56:17.0 +0100
> > > +++ gcc/omp-low.c 2015-11-20 13:44:29.080374051 +0100
> > > @@ -,11 +,13 @@ lower_rec_input_clauses (tree clauses, g
> > >  
> > > if (!integer_zerop (bias))
> > >   {
> > > -   bias = fold_convert_loc (clause_loc, sizetype, bias);
> > > -   bias = fold_build1_loc (clause_loc, NEGATE_EXPR,
> > > -   sizetype, bias);
> > > -   x = fold_build2_loc (clause_loc, POINTER_PLUS_EXPR,
> > > -TREE_TYPE (x), x, bias);
> > > +   bias = fold_convert_loc (clause_loc, pointer_sized_int_node,
> > > +bias);
> > > +   yb = fold_convert_loc (clause_loc, pointer_sized_int_node,
> > > +  x);
> > > +   yb = fold_build2_loc (clause_loc, MINUS_EXPR,
> > > + pointer_sized_int_node, yb, bias);
> > > +   x = fold_convert_loc (clause_loc, TREE_TYPE (x), yb);
> > > yb = create_tmp_var (ptype, name);
> > > gimplify_assign (yb, x, ilist);
> > > x = yb;
> 
> OK to commit the following to trunk?
> 
> commit 92b0eebfcbe914d3addeb97d4bb33f76a44dbe60
> Author: Thomas Schwinge 
> Date:   Thu Nov 26 14:21:13 2015 +0100
> 
> Restore original POINTER_PLUS_EXPR code
> 
>   PR middle-end/68221
>   gcc/
>   * omp-low.c (lower_rec_input_clauses): If C/C++ array reduction
>   has non-zero bias, use pointer plus of negated bias instead of
>   subtracting it in integer type.
> ---
>  gcc/omp-low.c |   12 +---
>  1 file changed, 5 insertions(+), 7 deletions(-)
> 
> diff --git gcc/omp-low.c gcc/omp-low.c
> index 0b44588..927d9d9 100644
> --- gcc/omp-low.c
> +++ gcc/omp-low.c
> @@ -4451,13 +4451,11 @@ lower_rec_input_clauses (tree clauses, gimple_seq 
> *ilist, gimple_seq *dlist,
>  
> if (!integer_zerop (bias))
>   {
> -   bias = fold_convert_loc (clause_loc, pointer_sized_int_node,
> -bias);
> -   yb = fold_convert_loc (clause_loc, pointer_sized_int_node,
> -  x);
> -   yb = fold_build2_loc (clause_loc, MINUS_EXPR,
> - pointer_sized_int_node, yb, bias);
> -   x = fold_convert_loc (clause_loc, TREE_TYPE (x), yb);
> +   bias = fold_convert_loc (clause_loc, sizetype, bias);
> +   bias = fold_build1_loc (clause_loc, NEGATE_EXPR,
> +   sizetype, bias);
> +   x = fold_build2_loc (clause_loc, POINTER_PLUS_EXPR,
> +TREE_TYPE (x), x, bias);
> yb = create_tmp_var (ptype, name);
> gimplify_assign (yb, x, ilist);
> x = yb;


Grüße
 Thomas


signature.asc
Description: PGP signature

Re: [PATCH] Fix up reduction-1{1,2} testcases (PR middle-end/68221)

[Richard Biener]

On Fri, 20 Nov 2015, Jakub Jelinek wrote:

> Hi!
> 
> If C/C++ array section reductions have non-zero (positive) bias, it is
> implemented by declaring a smaller private array and subtracting the bias
> from the start of the private array (because valid code may only dereference
> elements from bias onwards).  But, this isn't something that is kosher in
> C/C++ pointer arithmetics and the alias oracle seems to get upset on that.
> So, the following patch fixes that by performing the subtraction on integral
> type instead of p+ -bias.

So this still does use the biased pointer because you do not
re-write accesses (where you could have applied the biasing to
the indexes / offsets), right?  Thus the patch is merely obfuscation
for GCC rather than making it kosher for C/C++ (you still have a
pointer pointing outside of the private array object)?

I still hope to have a look where the alias oracle gets things
wrong (well, if so by accident at least).

Richard.

> Bootstrapped/regtested on x86_64-linux and i686-linux, committed to trunk.
> 
> 2015-11-20  Jakub Jelinek  
> 
>   PR middle-end/68221
>   * omp-low.c (lower_rec_input_clauses): If C/C++ array reduction
>   has non-zero bias, subtract it in integer type instead of
>   pointer plus of negated bias.
> 
>   * testsuite/libgomp.c/reduction-11.c: Remove xfail.
>   * testsuite/libgomp.c/reduction-12.c: Likewise.
>   * testsuite/libgomp.c++/reduction-11.C: Likewise.
>   * testsuite/libgomp.c++/reduction-12.C: Likewise.
> 
> --- gcc/omp-low.c.jj  2015-11-20 12:56:17.0 +0100
> +++ gcc/omp-low.c 2015-11-20 13:44:29.080374051 +0100
> @@ -,11 +,13 @@ lower_rec_input_clauses (tree clauses, g
>  
> if (!integer_zerop (bias))
>   {
> -   bias = fold_convert_loc (clause_loc, sizetype, bias);
> -   bias = fold_build1_loc (clause_loc, NEGATE_EXPR,
> -   sizetype, bias);
> -   x = fold_build2_loc (clause_loc, POINTER_PLUS_EXPR,
> -TREE_TYPE (x), x, bias);
> +   bias = fold_convert_loc (clause_loc, pointer_sized_int_node,
> +bias);
> +   yb = fold_convert_loc (clause_loc, pointer_sized_int_node,
> +  x);
> +   yb = fold_build2_loc (clause_loc, MINUS_EXPR,
> + pointer_sized_int_node, yb, bias);
> +   x = fold_convert_loc (clause_loc, TREE_TYPE (x), yb);
> yb = create_tmp_var (ptype, name);
> gimplify_assign (yb, x, ilist);
> x = yb;
> --- libgomp/testsuite/libgomp.c/reduction-11.c.jj 2015-11-05 
> 16:03:53.0 +0100
> +++ libgomp/testsuite/libgomp.c/reduction-11.c2015-11-20 
> 13:38:24.448520879 +0100
> @@ -1,4 +1,4 @@
> -/* { dg-do run { xfail *-*-* } } */
> +/* { dg-do run } */
>  
>  char z[10] = { 0 };
>  
> --- libgomp/testsuite/libgomp.c/reduction-12.c.jj 2015-11-05 
> 16:03:53.0 +0100
> +++ libgomp/testsuite/libgomp.c/reduction-12.c2015-11-20 
> 13:38:34.565378078 +0100
> @@ -1,4 +1,4 @@
> -/* { dg-do run { xfail *-*-* } } */
> +/* { dg-do run } */
>  
>  struct A { int t; };
>  struct B { char t; };
> --- libgomp/testsuite/libgomp.c++/reduction-11.C.jj   2015-11-05 
> 16:03:53.0 +0100
> +++ libgomp/testsuite/libgomp.c++/reduction-11.C  2015-11-20 
> 13:37:53.921951766 +0100
> @@ -1,4 +1,4 @@
> -// { dg-do run { xfail *-*-* } }
> +// { dg-do run }
>  
>  char z[10] = { 0 };
>  
> --- libgomp/testsuite/libgomp.c++/reduction-12.C.jj   2015-11-05 
> 16:03:53.0 +0100
> +++ libgomp/testsuite/libgomp.c++/reduction-12.C  2015-11-20 
> 13:38:03.983809741 +0100
> @@ -1,4 +1,4 @@
> -// { dg-do run { xfail *-*-* } }
> +// { dg-do run }
>  
>  template 
>  struct A
> 
>   Jakub
> 
> 

-- 
Richard Biener 
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 
21284 (AG Nuernberg)

[PATCH] Fix up reduction-1{1,2} testcases (PR middle-end/68221)

[Jakub Jelinek]

Hi!

If C/C++ array section reductions have non-zero (positive) bias, it is
implemented by declaring a smaller private array and subtracting the bias
from the start of the private array (because valid code may only dereference
elements from bias onwards).  But, this isn't something that is kosher in
C/C++ pointer arithmetics and the alias oracle seems to get upset on that.
So, the following patch fixes that by performing the subtraction on integral
type instead of p+ -bias.

Bootstrapped/regtested on x86_64-linux and i686-linux, committed to trunk.

2015-11-20  Jakub Jelinek  

PR middle-end/68221
* omp-low.c (lower_rec_input_clauses): If C/C++ array reduction
has non-zero bias, subtract it in integer type instead of
pointer plus of negated bias.

* testsuite/libgomp.c/reduction-11.c: Remove xfail.
* testsuite/libgomp.c/reduction-12.c: Likewise.
* testsuite/libgomp.c++/reduction-11.C: Likewise.
* testsuite/libgomp.c++/reduction-12.C: Likewise.

--- gcc/omp-low.c.jj2015-11-20 12:56:17.0 +0100
+++ gcc/omp-low.c   2015-11-20 13:44:29.080374051 +0100
@@ -,11 +,13 @@ lower_rec_input_clauses (tree clauses, g
 
  if (!integer_zerop (bias))
{
- bias = fold_convert_loc (clause_loc, sizetype, bias);
- bias = fold_build1_loc (clause_loc, NEGATE_EXPR,
- sizetype, bias);
- x = fold_build2_loc (clause_loc, POINTER_PLUS_EXPR,
-  TREE_TYPE (x), x, bias);
+ bias = fold_convert_loc (clause_loc, pointer_sized_int_node,
+  bias);
+ yb = fold_convert_loc (clause_loc, pointer_sized_int_node,
+x);
+ yb = fold_build2_loc (clause_loc, MINUS_EXPR,
+   pointer_sized_int_node, yb, bias);
+ x = fold_convert_loc (clause_loc, TREE_TYPE (x), yb);
  yb = create_tmp_var (ptype, name);
  gimplify_assign (yb, x, ilist);
  x = yb;
--- libgomp/testsuite/libgomp.c/reduction-11.c.jj   2015-11-05 
16:03:53.0 +0100
+++ libgomp/testsuite/libgomp.c/reduction-11.c  2015-11-20 13:38:24.448520879 
+0100
@@ -1,4 +1,4 @@
-/* { dg-do run { xfail *-*-* } } */
+/* { dg-do run } */
 
 char z[10] = { 0 };
 
--- libgomp/testsuite/libgomp.c/reduction-12.c.jj   2015-11-05 
16:03:53.0 +0100
+++ libgomp/testsuite/libgomp.c/reduction-12.c  2015-11-20 13:38:34.565378078 
+0100
@@ -1,4 +1,4 @@
-/* { dg-do run { xfail *-*-* } } */
+/* { dg-do run } */
 
 struct A { int t; };
 struct B { char t; };
--- libgomp/testsuite/libgomp.c++/reduction-11.C.jj 2015-11-05 
16:03:53.0 +0100
+++ libgomp/testsuite/libgomp.c++/reduction-11.C2015-11-20 
13:37:53.921951766 +0100
@@ -1,4 +1,4 @@
-// { dg-do run { xfail *-*-* } }
+// { dg-do run }
 
 char z[10] = { 0 };
 
--- libgomp/testsuite/libgomp.c++/reduction-12.C.jj 2015-11-05 
16:03:53.0 +0100
+++ libgomp/testsuite/libgomp.c++/reduction-12.C2015-11-20 
13:38:03.983809741 +0100
@@ -1,4 +1,4 @@
-// { dg-do run { xfail *-*-* } }
+// { dg-do run }
 
 template 
 struct A

Jakub