public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Thomas Schwinge <thomas@codesourcery.com>
To: Richard Biener <rguenther@suse.de>,
	Jakub Jelinek <jakub@redhat.com>,	<gcc-patches@gcc.gnu.org>
Subject: Re: [PATCH] Fix up reduction-1{1,2} testcases (PR middle-end/68221)
Date: Thu, 21 Jan 2016 06:18:00 -0000	[thread overview]
Message-ID: <87a8nzzca0.fsf@kepler.schwinge.homeip.net> (raw)
In-Reply-To: <87bn8s7611.fsf@kepler.schwinge.homeip.net>

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

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 472 bytes --]

  reply	other threads:[~2016-01-21  6:18 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-11-20 20:06 Jakub Jelinek
2015-11-23 11:15 ` Richard Biener
2015-11-26 13:37   ` Thomas Schwinge
2015-12-23 11:04     ` Thomas Schwinge
2016-01-11 10:41       ` Thomas Schwinge
2016-01-21  6:18         ` Thomas Schwinge [this message]
2016-01-22 10:18           ` Jakub Jelinek

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87a8nzzca0.fsf@kepler.schwinge.homeip.net \
    --to=thomas@codesourcery.com \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=jakub@redhat.com \
    --cc=rguenther@suse.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).