public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Richard Biener <rguenther@suse.de>
To: Chung-Lin Tang <cltang@codesourcery.com>
Cc: "Martin Liška" <mliska@suse.cz>, gcc-patches <gcc-patches@gcc.gnu.org>
Subject: Re: [patch] Fix PR fortran/72743
Date: Thu, 01 Sep 2016 07:14:00 -0000	[thread overview]
Message-ID: <alpine.LSU.2.11.1609010905070.26629@t29.fhfr.qr> (raw)
In-Reply-To: <2ebcf9da-519e-e2c7-0a52-779c96323496@codesourcery.com>

On Wed, 31 Aug 2016, Chung-Lin Tang wrote:

> Hi Richard, Martin,
> this issue is actually sort of like PR 70856, basically the same ICE
> after IPA-ICF, due to DECL_PT_UIDs not consistent after reaching for the
> ultimate_alias_target().
> 
> The reason this wasn't covered by the PR70856 fix is that, in this case,
> the DECL_PT_UID was not set in original->decl, evading the condition, and
> hence the the merged alias doesn't have DECL_PT_UID set, and causes the
> ICE in the second round of IPA-PTA (under -fopenacc).
> 
> My fix is to simply remove the DECL_PT_UID_SET_P (original->decl) guard,
> and allow the DECL_PT_UID to be set using the DECL_UID in this case.
> 
> Does this fix make sense?
> Testing does show no regressions, and the PR testcase ICE is fixed.

If original is the ultimate alias target then this is indeed correct.

The IPA PTA code asserts that all decls that are an alias of the ultimate
alias target X have the same DECL_PT_UID as X DECL_UID (or if the new 
alias doesn't have DECL_PT_UID set it will set it that way).

So I'd say it should be

  SET_DECL_PT_UID (alias->decl, DECL_UID (original->decl));

instead.

If IPA-ICF would merge

static const int x = 1;
static const int xa __attribute__((alias("x")));
static const int y = 1;
static const int ya __attribute__((alias("y")));

we'd initially have DECL_PT_UID (xa) be DECL_UID (x) and
DECL_PT_UID (ya) be DECL_UID (y).  Depending on how exactly IPA-ICF
performs the merging of x and y (and thus adjusts the existing aliases)
IPA-ICF needs to adjust DECL_PT_UID of all aliases of the new
"ultimate alias target".  I don't think the current code with the
proposed patch makes sure this will happen.

Thanks,
Richard.

> Thanks,
> Chung-Lin
> 
> 	PR fortran/72743
> 	* ipa-icf.c (sem_variable::merge): Remove guard condition for
> 	setting DECL_PT_UID (alias->decl).
> 
> 	testsuite/
> 	* gfortran.dg/goacc/pr72743.f90: New test.
> 
> 
> 

-- 
Richard Biener <rguenther@suse.de>
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nuernberg)

  reply	other threads:[~2016-09-01  7:14 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-08-31 10:10 Chung-Lin Tang
2016-09-01  7:14 ` Richard Biener [this message]
2016-09-09 14:33   ` Chung-Lin Tang
2016-09-14  9:00     ` Richard Biener

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=alpine.LSU.2.11.1609010905070.26629@t29.fhfr.qr \
    --to=rguenther@suse.de \
    --cc=cltang@codesourcery.com \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=mliska@suse.cz \
    /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).