From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 49484 invoked by alias); 1 Jul 2016 06:58:51 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 49472 invoked by uid 89); 1 Jul 2016 06:58:50 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.6 required=5.0 tests=AWL,BAYES_00,KAM_LAZY_DOMAIN_SECURITY,RCVD_IN_DNSWL_NONE,RP_MATCHES_RCVD autolearn=ham version=3.3.2 spammy=symtab, sk:FOR_EAC, uncovered, sk:for_eac X-HELO: smtp.ispras.ru Received: from smtp.ispras.ru (HELO smtp.ispras.ru) (83.149.199.79) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Fri, 01 Jul 2016 06:58:48 +0000 Received: from [10.10.3.121] (unknown [83.149.199.91]) by smtp.ispras.ru (Postfix) with ESMTP id 635672040E; Fri, 1 Jul 2016 09:58:45 +0300 (MSK) Date: Fri, 01 Jul 2016 06:58:00 -0000 From: Alexander Monakov To: Jan Hubicka cc: gcc-patches@gcc.gnu.org Subject: Re: [PATCH] Handle undefined extern vars in output_in_order In-Reply-To: Message-ID: References: <20160616142524.GA93274@kam.mff.cuni.cz> <20160616152403.GA38925@kam.mff.cuni.cz> <20160616154239.GA416@kam.mff.cuni.cz> User-Agent: Alpine 2.20 (LNX 67 2015-01-07) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII X-SW-Source: 2016-07/txt/msg00003.txt.bz2 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. > (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; > >