From: Alexander Monakov <amonakov@ispras.ru>
To: Jan Hubicka <hubicka@ucw.cz>
Cc: gcc-patches@gcc.gnu.org
Subject: Re: [PATCH] Handle undefined extern vars in output_in_order
Date: Fri, 08 Jul 2016 07:47:00 -0000 [thread overview]
Message-ID: <alpine.LNX.2.20.13.1607081046161.25308@monopod.intra.ispras.ru> (raw)
In-Reply-To: <alpine.LNX.2.20.1607010958240.29735@monopod.intra.ispras.ru>
On Fri, 1 Jul 2016, Alexander Monakov wrote:
> On Thu, 23 Jun 2016, Alexander Monakov wrote:
> > Hi,
> >
> > I've discovered that this assert in my patch was too restrictive:
> >
> > + if (DECL_HAS_VALUE_EXPR_P (pv->decl))
> > + {
> > + gcc_checking_assert (lookup_attribute ("omp declare target link",
> > + DECL_ATTRIBUTES (pv->decl)));
> >
> > Testing for the nvptx target uncovered that there's another case where a
> > global variable would have a value expr: emutls. Sorry for not spotting it
> > earlier (but at least the new assert did its job). I think we should always
> > skip here over decls that have value-exprs, just like hard-reg vars are
> > skipped. The following patch does that. Is this still OK?
>
> Ping.
Ping^2.
> > (bootstrapped/regtested on x86-64)
> >
> > Alexander
> >
> > * cgraphunit.c (cgraph_order_sort_kind): New entry ORDER_VAR_UNDEF.
> > (output_in_order): Loop over undefined variables too. Output them
> > via assemble_undefined_decl. Skip variables that correspond to hard
> > registers or have value-exprs.
> > * varpool.c (symbol_table::output_variables): Handle undefined
> > variables together with defined ones.
> >
> > diff --git a/gcc/cgraphunit.c b/gcc/cgraphunit.c
> > index 4bfcad7..e30fe6e 100644
> > --- a/gcc/cgraphunit.c
> > +++ b/gcc/cgraphunit.c
> > @@ -2141,6 +2141,7 @@ enum cgraph_order_sort_kind
> > ORDER_UNDEFINED = 0,
> > ORDER_FUNCTION,
> > ORDER_VAR,
> > + ORDER_VAR_UNDEF,
> > ORDER_ASM
> > };
> >
> > @@ -2187,16 +2188,20 @@ output_in_order (bool no_reorder)
> > }
> > }
> >
> > - FOR_EACH_DEFINED_VARIABLE (pv)
> > - if (!DECL_EXTERNAL (pv->decl))
> > - {
> > - if (no_reorder && !pv->no_reorder)
> > - continue;
> > - i = pv->order;
> > - gcc_assert (nodes[i].kind == ORDER_UNDEFINED);
> > - nodes[i].kind = ORDER_VAR;
> > - nodes[i].u.v = pv;
> > - }
> > + /* There is a similar loop in symbol_table::output_variables.
> > + Please keep them in sync. */
> > + FOR_EACH_VARIABLE (pv)
> > + {
> > + if (no_reorder && !pv->no_reorder)
> > + continue;
> > + if (DECL_HARD_REGISTER (pv->decl)
> > + || DECL_HAS_VALUE_EXPR_P (pv->decl))
> > + continue;
> > + i = pv->order;
> > + gcc_assert (nodes[i].kind == ORDER_UNDEFINED);
> > + nodes[i].kind = pv->definition ? ORDER_VAR : ORDER_VAR_UNDEF;
> > + nodes[i].u.v = pv;
> > + }
> >
> > for (pa = symtab->first_asm_symbol (); pa; pa = pa->next)
> > {
> > @@ -2222,16 +2227,13 @@ output_in_order (bool no_reorder)
> > break;
> >
> > case ORDER_VAR:
> > -#ifdef ACCEL_COMPILER
> > - /* Do not assemble "omp declare target link" vars. */
> > - if (DECL_HAS_VALUE_EXPR_P (nodes[i].u.v->decl)
> > - && lookup_attribute ("omp declare target link",
> > - DECL_ATTRIBUTES (nodes[i].u.v->decl)))
> > - break;
> > -#endif
> > nodes[i].u.v->assemble_decl ();
> > break;
> >
> > + case ORDER_VAR_UNDEF:
> > + assemble_undefined_decl (nodes[i].u.v->decl);
> > + break;
> > +
> > case ORDER_ASM:
> > assemble_asm (nodes[i].u.a->asm_str);
> > break;
> > diff --git a/gcc/varpool.c b/gcc/varpool.c
> > index ab615fa..e5f991e 100644
> > --- a/gcc/varpool.c
> > +++ b/gcc/varpool.c
> > @@ -733,11 +733,6 @@ symbol_table::output_variables (void)
> >
> > timevar_push (TV_VAROUT);
> >
> > - FOR_EACH_VARIABLE (node)
> > - if (!node->definition
> > - && !DECL_HAS_VALUE_EXPR_P (node->decl)
> > - && !DECL_HARD_REGISTER (node->decl))
> > - assemble_undefined_decl (node->decl);
> > FOR_EACH_DEFINED_VARIABLE (node)
> > {
> > /* Handled in output_in_order. */
> > @@ -747,20 +742,19 @@ symbol_table::output_variables (void)
> > node->finalize_named_section_flags ();
> > }
> >
> > - FOR_EACH_DEFINED_VARIABLE (node)
> > + /* There is a similar loop in output_in_order. Please keep them in sync. */
> > + FOR_EACH_VARIABLE (node)
> > {
> > /* Handled in output_in_order. */
> > if (node->no_reorder)
> > continue;
> > -#ifdef ACCEL_COMPILER
> > - /* Do not assemble "omp declare target link" vars. */
> > - if (DECL_HAS_VALUE_EXPR_P (node->decl)
> > - && lookup_attribute ("omp declare target link",
> > - DECL_ATTRIBUTES (node->decl)))
> > + if (DECL_HARD_REGISTER (node->decl)
> > + || DECL_HAS_VALUE_EXPR_P (node->decl))
> > continue;
> > -#endif
> > - if (node->assemble_decl ())
> > - changed = true;
> > + if (node->definition)
> > + changed |= node->assemble_decl ();
> > + else
> > + assemble_undefined_decl (node->decl);
> > }
> > timevar_pop (TV_VAROUT);
> > return changed;
> >
> >
next prev parent reply other threads:[~2016-07-08 7:47 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-06-09 13:13 Alexander Monakov
2016-06-16 13:59 ` Alexander Monakov
2016-06-16 14:25 ` Jan Hubicka
2016-06-16 14:37 ` Alexander Monakov
2016-06-16 15:24 ` Jan Hubicka
2016-06-16 15:36 ` Alexander Monakov
2016-06-16 15:42 ` Jan Hubicka
2016-06-23 14:45 ` Alexander Monakov
2016-07-01 6:58 ` Alexander Monakov
2016-07-08 7:47 ` Alexander Monakov [this message]
2016-07-14 18:08 ` Jeff Law
2016-07-14 18:34 ` Alexander Monakov
2016-07-14 20:28 ` Jeff Law
2016-07-14 20:31 ` Jeff Law
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.LNX.2.20.13.1607081046161.25308@monopod.intra.ispras.ru \
--to=amonakov@ispras.ru \
--cc=gcc-patches@gcc.gnu.org \
--cc=hubicka@ucw.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).