| Message ID | 8737vsrinn.fsf@kepler.schwinge.homeip.net |
|---|---|
| State | New |
| Headers | show |
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 <rguenther@suse.de> 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, > <https://gcc.gnu.org/bugzilla/show_bug.cgi?id=68221#c5>? > > 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.000000000 +0100 > > > +++ gcc/omp-low.c 2015-11-20 13:44:29.080374051 +0100 > > > @@ -4444,11 +4444,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 <thomas@codesourcery.com> > 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
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 <rguenther@suse.de> 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, > > <https://gcc.gnu.org/bugzilla/show_bug.cgi?id=68221#c5>? > > > > 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.000000000 +0100 > > > > +++ gcc/omp-low.c 2015-11-20 13:44:29.080374051 +0100 > > > > @@ -4444,11 +4444,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 <thomas@codesourcery.com> > > 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
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 <rguenther@suse.de> 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, > > > <https://gcc.gnu.org/bugzilla/show_bug.cgi?id=68221#c5>? > > > > > > 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.000000000 +0100 > > > > > +++ gcc/omp-low.c 2015-11-20 13:44:29.080374051 +0100 > > > > > @@ -4444,11 +4444,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 <thomas@codesourcery.com> > > > 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
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
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;