Re: [PATCH] Handle undefined extern vars in output_in_order

[Alexander Monakov <amonakov at ispras dot ru>]

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;
> 
>