Handle undefined extern vars in output_in_order

Hi,

This patch teaches cgraphunit.c:output_in_order to output undefined external
variables via assemble_undefined_decl.  At the moment that is only done for
-ftoplevel-reorder in varpool.c:symbol_table::output_variables.  This patch
makes both behave the same way.  I've also made handling of variables in both
functions look similar to each other.

I'll admit it's rather unclear to me what the '!DECL_EXTERNAL (pv->decl)'
condition is guarding in output_in_order (removed in my patch).  AIUI, it
should be always true for defined variables (or, conversely, if it's false
for a defined variable, why are we skipping it?).

I have plans to reimplement handling of "omp declare target link" in a more
clean manner.  Now it looks quite out of place there.

Bootstrapped and regtested on x86_64, OK for trunk?

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 "omp declare target link" variables
	earlier.
	* varpool.c (symbol_table::output_variables): Handle undefined
	variables together with defined ones.

On Thu, 9 Jun 2016, Alexander Monakov wrote:

> Hi,
> 
> This patch teaches cgraphunit.c:output_in_order to output undefined external
> variables via assemble_undefined_decl.  At the moment that is only done for
> -ftoplevel-reorder in varpool.c:symbol_table::output_variables.  This patch
> makes both behave the same way.  I've also made handling of variables in both
> functions look similar to each other.

Ping.

Thanks.
Alexander

> On Thu, 9 Jun 2016, Alexander Monakov wrote:
> 
> > Hi,
> > 
> > This patch teaches cgraphunit.c:output_in_order to output undefined external
> > variables via assemble_undefined_decl.  At the moment that is only done for
> > -ftoplevel-reorder in varpool.c:symbol_table::output_variables.  This patch
> > makes both behave the same way.  I've also made handling of variables in both
> > functions look similar to each other.
> 
> Ping.
+  FOR_EACH_VARIABLE (pv)
+    {
+      if (no_reorder && !pv->no_reorder)
+	continue;
+      if (DECL_HARD_REGISTER (pv->decl))
+	continue;
+      if (DECL_HAS_VALUE_EXPR_P (pv->decl))
+	{
+	  gcc_checking_assert (lookup_attribute ("omp declare target link",
+						 DECL_ATTRIBUTES (pv->decl)));
+#ifdef ACCEL_COMPILER
+	  continue;
+#endif
+	}
+      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;

order for undefined variables is not computed, so it will be 0. Doesn't think overwrite existing
entries of nodes array?

Honza
> 
> Thanks.
> Alexander

On Thu, 16 Jun 2016, Jan Hubicka wrote:
> > On Thu, 9 Jun 2016, Alexander Monakov wrote:
> +  FOR_EACH_VARIABLE (pv)
[snip]
> +      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;
> 
> order for undefined variables is not computed, so it will be 0. Doesn't
> think overwrite existing entries of nodes array?

Hm, I've tried the following testcase:

extern int a, b;
int f()
{
  return a+b;
}

and in the above loop I see pv->order == 2 on the first iteration,
pv->order == 1 on the second. Under what circumstances wouldn't
order be computed?

Thanks.
Alexander

> On Thu, 16 Jun 2016, Jan Hubicka wrote:
> > > On Thu, 9 Jun 2016, Alexander Monakov wrote:
> > +  FOR_EACH_VARIABLE (pv)
> [snip]
> > +      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;
> > 
> > order for undefined variables is not computed, so it will be 0. Doesn't
> > think overwrite existing entries of nodes array?
> 
> Hm, I've tried the following testcase:
> 
> extern int a, b;
> int f()
> {
>   return a+b;
> }
> 
> and in the above loop I see pv->order == 2 on the first iteration,
> pv->order == 1 on the second. Under what circumstances wouldn't
> order be computed?

I see, order is created at a time variable is added to symbol table (not at time when definition is given).
So we should have order everywhere.
Patch is OK

Honza
> 
> Thanks.
> Alexander

On Thu, 16 Jun 2016, Jan Hubicka wrote:
> I see, order is created at a time variable is added to symbol table (not at
> time when definition is given).  So we should have order everywhere.
> Patch is OK

Thanks!  If you don't mind a quick followup question: now that both
FOR_EACH_VARIABLE loops in two functions have the same structure, is
it alright to add a comment of the form

  /* There is a similar loop in output_in_order.  Please keep them in sync.  */

to symbol_table::output_variables, and vice versa?

Alexander

> On Thu, 16 Jun 2016, Jan Hubicka wrote:
> > I see, order is created at a time variable is added to symbol table (not at
> > time when definition is given).  So we should have order everywhere.
> > Patch is OK
> 
> Thanks!  If you don't mind a quick followup question: now that both
> FOR_EACH_VARIABLE loops in two functions have the same structure, is
> it alright to add a comment of the form
> 
>   /* There is a similar loop in output_in_order.  Please keep them in sync.  */
> 
> to symbol_table::output_variables, and vice versa?

Yes, that is fine.

Honza