HPPA constructor merge patch, PR middle-end/45388

HPPA constructor merge patch, PR middle-end/45388

[Steve Ellcey]

A lot of C++ tests started failing on HPPA with r163443, the constructor
merge patch.  HPPA (in 32 bit mode) does not have .ctors and .dtors
sections so it uses the collect2 magic name functionality to handle
static constructors.

In ipa.c at build_cdtor_fns the code says:

/* Generate functions to call static constructors and destructors
   for targets that do not support .ctors/.dtors sections.  These
   functions have magic names which are detected by collect2.  */

The problem is that while build_cdtor turns off DECL_STATIC_CONSTRUCTOR
(and DECL_STATIC_DESTRUCTOR) it does net set TREE_PUBLIC to 1 for these
functions and it is TREE_PUBLIC that is used in
ASM_DECLARE_FUNCTION_NAME to decide whether or not to make the
constructor or destructor name externally visible.  If it is not
externally visible then collect2 will not find the names in order to
set up the needed constructors.

Tested on hppa2.0w-hp-hpux11.11 (32 bit mode) with no regressions.
OK to checkin?

Steve Ellcey
sje@cup.hp.com


2010-09-27  Steve Ellcey  <sje@cup.hp.com>

	PR middle-end/45388
	* ipa.c: Set TREE_PUBLIC on constructors/destructors.


Index: ipa.c
===================================================================
--- ipa.c	(revision 164643)
+++ ipa.c	(working copy)
@@ -1477,6 +1477,7 @@ build_cdtor (bool ctor_p, VEC (tree, hea
 	    DECL_STATIC_CONSTRUCTOR (fn) = 0;
 	  else
 	    DECL_STATIC_DESTRUCTOR (fn) = 0;
+	  TREE_PUBLIC (fn) = 1;
 	  /* We do not want to optimize away pure/const calls here.
 	     When optimizing, these should be already removed, when not
 	     optimizing, we want user to be able to breakpoint in them.  */

Re: HPPA constructor merge patch, PR middle-end/45388

[Richard Henderson]

On 09/27/2010 01:29 PM, Steve Ellcey wrote:
> 	PR middle-end/45388
> 	* ipa.c: Set TREE_PUBLIC on constructors/destructors.

Not ok.  Constructors should not be public when we
do have .ctor/.dtor support.

This is supposed to be handled in cgraph_build_static_cdtor,

  if (!targetm.have_ctors_dtors)
    {
      TREE_PUBLIC (decl) = 1;
      DECL_PRESERVE_P (decl) = 1;
    }

there.  Can you figure out if that bit isn't being triggered?


r~

Re: HPPA constructor merge patch, PR middle-end/45388

[Steve Ellcey]

On Mon, 2010-09-27 at 13:33 -0700, Richard Henderson wrote:
> On 09/27/2010 01:29 PM, Steve Ellcey wrote:
> > 	PR middle-end/45388
> > 	* ipa.c: Set TREE_PUBLIC on constructors/destructors.
> 
> Not ok.  Constructors should not be public when we
> do have .ctor/.dtor support.
> 
> This is supposed to be handled in cgraph_build_static_cdtor,
> 
>   if (!targetm.have_ctors_dtors)
>     {
>       TREE_PUBLIC (decl) = 1;
>       DECL_PRESERVE_P (decl) = 1;
>     }
> 
> there.  Can you figure out if that bit isn't being triggered?
> 
> 
> r~

It is being triggered, but for a different routine.  In build_cdtor we
are setting TREE_PUBLIC on _GLOBAL__I__ZN2c12f6Ev.  In
cgraph_build_static_cdtor we are setting TREE_PUBLIC on
_GLOBAL__I_65535_0__ZN2c12f6Ev.  This is for the test case
g++.dg/abi/covariant3.C.

When I look at the code, I see that _GLOBAL__I__ZN2c12f6Ev is never
(directly) called by anything but it calls
_Z41__static_initialization_and_destruction_0ii, which
_GLOBAL__I_65535_0__ZN2c12f6Ev also calls.  Is _GLOBAL__I__ZN2c12f6Ev
the old (premerged) constructor that should no longer be used and has
been replaced by GLOBAL__I_65535_0__ZN2c12f6Ev?  In this case we
wouldn't want it global and collect2 should not see or call it, but
because the HP nm/linker/whatever HP-UX collect2 uses to find global
names sees it, we still try to call it even when we shouldn't.  But if
it has essentially been replaced, why wasn't it just removed entirely?

Steve Ellcey
sje@cup.hp.com

Re: HPPA constructor merge patch, PR middle-end/45388

[Richard Henderson]

On 09/27/2010 02:05 PM, Steve Ellcey wrote:
> When I look at the code, I see that _GLOBAL__I__ZN2c12f6Ev is never
> (directly) called by anything but it calls
> _Z41__static_initialization_and_destruction_0ii, which
> _GLOBAL__I_65535_0__ZN2c12f6Ev also calls.  Is _GLOBAL__I__ZN2c12f6Ev
> the old (premerged) constructor that should no longer be used and has
> been replaced by GLOBAL__I_65535_0__ZN2c12f6Ev?

Yes.  I believe that the expected full sequence is that 

  GLOBAL__I_65535_0__ZN2c12f6Ev calls
    GLOBAL__I__ZN2c12f6Ev calls
      _Z41__static_initialization_and_destruction_0ii

and that GLOBAL__I__ZN2c12f6Ev got inlined.

> But if
> it has essentially been replaced, why wasn't it just removed entirely?

An excellent question.  I wonder if it's still marked
as preserved or something?

Of course regardless it seems like we're absolutely
relying on that middle call being inlined and eliminated.
I can think of two solutions: adjust collect2 to ignore
local symbols, or rename the middle function so that it
no longer matches the collect2 pattern.  Both solutions
seem slightly unpleasent, actually.

Thoughts, Jan?


r~

Re: HPPA constructor merge patch, PR middle-end/45388

[Jan Hubicka]

> On 09/27/2010 02:05 PM, Steve Ellcey wrote:
> > When I look at the code, I see that _GLOBAL__I__ZN2c12f6Ev is never
> > (directly) called by anything but it calls
> > _Z41__static_initialization_and_destruction_0ii, which
> > _GLOBAL__I_65535_0__ZN2c12f6Ev also calls.  Is _GLOBAL__I__ZN2c12f6Ev
> > the old (premerged) constructor that should no longer be used and has
> > been replaced by GLOBAL__I_65535_0__ZN2c12f6Ev?
> 
> Yes.  I believe that the expected full sequence is that 
> 
>   GLOBAL__I_65535_0__ZN2c12f6Ev calls
>     GLOBAL__I__ZN2c12f6Ev calls
>       _Z41__static_initialization_and_destruction_0ii
> 
> and that GLOBAL__I__ZN2c12f6Ev got inlined.
> 
> > But if
> > it has essentially been replaced, why wasn't it just removed entirely?
> 
> An excellent question.  I wonder if it's still marked
> as preserved or something?
> 
> Of course regardless it seems like we're absolutely
> relying on that middle call being inlined and eliminated.
> I can think of two solutions: adjust collect2 to ignore
> local symbols, or rename the middle function so that it
> no longer matches the collect2 pattern.  Both solutions
> seem slightly unpleasent, actually.
> 
> Thoughts, Jan?

The code was setting disregard inline limits.  This is wrong as constructors
might contain stuff that is not really inlinable (and on LTO we might just blow
ourselves off by constructing overly huge functions).

I will take a look why it is not getting inlined in this particular case.  We might
just run off the large function growth limits or something.

But indeed I don't think we should have correctness depending on fact whether
we decided to inline or not.  My plan was to teach collect2 to ignore static symbols.
I didn't get to do that for a while, that is moslty because I am not terribly familiar
with the code and there was always always other problems to look into, but this seems
like correct solution to me.

Honza
> 
> 
> r~

Re: HPPA constructor merge patch, PR middle-end/45388

[John David Anglin]

> I will take a look why it is not getting inlined in this particular case.  We might
> just run off the large function growth limits or something.

I believe that it was inlined but a copy was preserved.  The copy was
not called internally.

> But indeed I don't think we should have correctness depending on fact whether
> we decided to inline or not.  My plan was to teach collect2 to ignore static symbols.
> I didn't get to do that for a while, that is moslty because I am not terribly familiar
> with the code and there was always always other problems to look into, but this seems
> like correct solution to me.

It's certainly a bug that collect2 doesn't ignore static symbols as
there is no way collect2 can arrange to call a static function from
an external object.  However, I also think it's a mistake to use the
magic names which are detected by collect2 for static constructors
and destructors.  The names detected by collect2 should always have
an encoded priority, etc.

Dave
-- 
J. David Anglin                                  dave.anglin@nrc-cnrc.gc.ca
National Research Council of Canada              (613) 990-0752 (FAX: 952-6602)

Re: HPPA constructor merge patch, PR middle-end/45388

[Jan Hubicka]

> > I will take a look why it is not getting inlined in this particular case.  We might
> > just run off the large function growth limits or something.
> 
> I believe that it was inlined but a copy was preserved.  The copy was
> not called internally.

Ah, I see, it is bug then, I will take a look into it.
> 
> > But indeed I don't think we should have correctness depending on fact whether
> > we decided to inline or not.  My plan was to teach collect2 to ignore static symbols.
> > I didn't get to do that for a while, that is moslty because I am not terribly familiar
> > with the code and there was always always other problems to look into, but this seems
> > like correct solution to me.
> 
> It's certainly a bug that collect2 doesn't ignore static symbols as
> there is no way collect2 can arrange to call a static function from
> an external object.  However, I also think it's a mistake to use the
> magic names which are detected by collect2 for static constructors
> and destructors.  The names detected by collect2 should always have
> an encoded priority, etc.

This is bit difficult to arrange with LTO.  We produce at compile time the consturctor
function with magic names, then at LTO time we want to produce single constructor function
calling them all.   We would need to guess on what name is the magic name (by same logic
as what collect does) and rename function back.
I can definitly implement it, but it seems more hackish than the collect2 side change.

Honza
> 
> Dave
> -- 
> J. David Anglin                                  dave.anglin@nrc-cnrc.gc.ca
> National Research Council of Canada              (613) 990-0752 (FAX: 952-6602)

Re: HPPA constructor merge patch, PR middle-end/45388

[Richard Henderson]

On 09/28/2010 08:36 AM, Jan Hubicka wrote:
> This is bit difficult to arrange with LTO.  We produce at compile time the consturctor
> function with magic names, then at LTO time we want to produce single constructor function
> calling them all.   We would need to guess on what name is the magic name (by same logic
> as what collect does) and rename function back.
> I can definitly implement it, but it seems more hackish than the collect2 side change.

An alternative is that at compile time we emit

  _Z41__static_initialization_and_destruction_0ii

to the intermediate code as the constructor, and

  GLOBAL__I__ZN2c12f6Ev calls

to the object code calling _Z41.  However, we don't emit
GLOBAL to the intermediate code at all.  Thus when LTO 
replaces the object files there's no trace of the original
GLOBAL function at all, and thus collect2 does not see it.
LTO will simply see _Z41 and call that function directly.

This is not entirely different from the case in which we
have .ctor support -- it's not like we read in the piece
of the object code that contains the .ctor data.  Just
consider the GLOBAL function object file data.

This should be doable with a flag on the decl for GLOBAL
that indicates that it should not be serialized.


r~

Re: HPPA constructor merge patch, PR middle-end/45388

[Jan Hubicka]

> On 09/28/2010 08:36 AM, Jan Hubicka wrote:
> > This is bit difficult to arrange with LTO.  We produce at compile time the consturctor
> > function with magic names, then at LTO time we want to produce single constructor function
> > calling them all.   We would need to guess on what name is the magic name (by same logic
> > as what collect does) and rename function back.
> > I can definitly implement it, but it seems more hackish than the collect2 side change.
> 
> An alternative is that at compile time we emit
> 
>   _Z41__static_initialization_and_destruction_0ii
> 
> to the intermediate code as the constructor, and
> 
>   GLOBAL__I__ZN2c12f6Ev calls
> 
> to the object code calling _Z41.  However, we don't emit
> GLOBAL to the intermediate code at all.  Thus when LTO 
> replaces the object files there's no trace of the original
> GLOBAL function at all, and thus collect2 does not see it.
> LTO will simply see _Z41 and call that function directly.
> 
> This is not entirely different from the case in which we
> have .ctor support -- it's not like we read in the piece
> of the object code that contains the .ctor data.  Just
> consider the GLOBAL function object file data.
> 
> This should be doable with a flag on the decl for GLOBAL
> that indicates that it should not be serialized.

Or we can just produce those collected global constructors after
serialization.  I will check...

Honza
> 
> 
> r~

Re: HPPA constructor merge patch, PR middle-end/45388

[John David Anglin]

On Tue, 28 Sep 2010, Jan Hubicka wrote:

> Or we can just produce those collected global constructors after
> serialization.  I will check...

Any progress in fixing this?

Dave
-- 
J. David Anglin                                  dave.anglin@nrc-cnrc.gc.ca
National Research Council of Canada              (613) 990-0752 (FAX: 952-6602)

Re: HPPA constructor merge patch, PR middle-end/45388

[Steve Ellcey]

On 09/28/2010 18:30 AM, Jan Hubicka wrote:

> > On 09/28/2010 08:36 AM, Jan Hubicka wrote:
> > > This is bit difficult to arrange with LTO.  We produce at compile time the consturctor
> > > function with magic names, then at LTO time we want to produce single constructor function
> > > calling them all.   We would need to guess on what name is the magic name (by same logic
> > > as what collect does) and rename function back.
> > > I can definitly implement it, but it seems more hackish than the collect2 side change.
> > 
> > An alternative is that at compile time we emit
> > 
> >   _Z41__static_initialization_and_destruction_0ii
> > 
> > to the intermediate code as the constructor, and
> > 
> >   GLOBAL__I__ZN2c12f6Ev calls
> > 
> > to the object code calling _Z41.  However, we don't emit
> > GLOBAL to the intermediate code at all.  Thus when LTO 
> > replaces the object files there's no trace of the original
> > GLOBAL function at all, and thus collect2 does not see it.
> > LTO will simply see _Z41 and call that function directly.
> > 
> > This is not entirely different from the case in which we
> > have .ctor support -- it's not like we read in the piece
> > of the object code that contains the .ctor data.  Just
> > consider the GLOBAL function object file data.
> > 
> > This should be doable with a flag on the decl for GLOBAL
> > that indicates that it should not be serialized.
> 
> Or we can just produce those collected global constructors after
> serialization.  I will check...
> 
> Honza
> > 
> > 
> > r~

Honza,

Have you looked into this any more?  I haven't seen any follow up from
you since this email.  This defect, PR middle-end/45388, is a P1 defect,
it breaks the hppa bootstrap, and it has been open for over a month.  I
would like to see this fixed, or your patch reverted, before we release
4.6.

Steve Ellcey
sje@cup.hp.com

Re: HPPA constructor merge patch, PR middle-end/45388

[John David Anglin]

[-- Attachment #1: Type: text/plain, Size: 2833 bytes --]

The attached change reverts commit r163443.  This commit broke
constructor/destructor handling on targets using the collect2
_GLOBAL_* mechanism (e.g., 32-bit hppa*-*-hpux*).

It has now been nearly four months since the commit was made and
no progress has been made in finding a fix along the lines discussed
below.  I believe the change should be reverted to provide an
incentive to fix the issue.

With the change in place, it causes approximately 155 g++, 74 obj-c++
and 5 libgomp fails.  Thus, it is time consuming to watch for new
regressions in test runs.

The reversion is clean.  Except for the changes to ipa.c, the commit
can be directly reverted.  For ipa.c, there were some subsequent
changes to the new code but no interaction between old and new.
At this time, the change can be easily restored if a solution to
the collect2 problem is found.

The reversion has been tested on hppa2.0w-hp-hpux11.11.  It fixes
all the fails introduced in r163443.  There is one regression:
gcc.dg/ipa/ctor-empty-1.c.  This is expected.  See
http://gcc.gnu.org/ml/gcc-testresults/2010-12/msg01017.html
for test results.

Ok to commit?

On Tue, 28 Sep 2010, Jan Hubicka wrote:

> > On 09/28/2010 08:36 AM, Jan Hubicka wrote:
> > > This is bit difficult to arrange with LTO.  We produce at compile time the consturctor
> > > function with magic names, then at LTO time we want to produce single constructor function
> > > calling them all.   We would need to guess on what name is the magic name (by same logic
> > > as what collect does) and rename function back.
> > > I can definitly implement it, but it seems more hackish than the collect2 side change.
> > 
> > An alternative is that at compile time we emit
> > 
> >   _Z41__static_initialization_and_destruction_0ii
> > 
> > to the intermediate code as the constructor, and
> > 
> >   GLOBAL__I__ZN2c12f6Ev calls
> > 
> > to the object code calling _Z41.  However, we don't emit
> > GLOBAL to the intermediate code at all.  Thus when LTO 
> > replaces the object files there's no trace of the original
> > GLOBAL function at all, and thus collect2 does not see it.
> > LTO will simply see _Z41 and call that function directly.
> > 
> > This is not entirely different from the case in which we
> > have .ctor support -- it's not like we read in the piece
> > of the object code that contains the .ctor data.  Just
> > consider the GLOBAL function object file data.
> > 
> > This should be doable with a flag on the decl for GLOBAL
> > that indicates that it should not be serialized.
> 
> Or we can just produce those collected global constructors after
> serialization.  I will check...
> 
> Honza
> > 
> > 
> > r~

Dave
-- 
J. David Anglin                                  dave.anglin@nrc-cnrc.gc.ca
National Research Council of Canada              (613) 990-0752 (FAX: 952-6602)

[-- Attachment #2: r163443.d.1 --]
[-- Type: text/plain, Size: 19618 bytes --]

2010-12-12  John David Anglin  <dave.anglin@nrc-cnrc.gc.ca>

	PR middle-end/45388
	Revert:
	2010-08-20  Jan Hubicka  <jh@suse.cz>

	* tree-pass.h (pass_ipa_cdtor_merge): New function.
	* cgraphunit.c (static_ctors, static_dtors): Move to ipa.c; make
	heap allocated.
	(record_cdtor_fn): Move to ipa.c; do not test for
	have_ctors_dtors.
	(build_cdtor): Move to ipa.c; add code avoiding construction
	when target have ctors/dtors and there is only one ctor/dtor at given
	priority.
	(compare_ctor, compare_dtor): Move to ipa.c; use DECL_UID to stabilize
	sort; reverse order of constructors.
	(cgraph_build_cdtor_fns): Move to ipa.c; rename to build_cdtor_fns.
	(cgraph_finalize_function): Do not call record_cdtor_fn.
	(cgraph_finalize_compilation_unit): Do not call cgraph_build_cdtor_fns.
	(cgraph_build_static_cdtor): Move to ipa.c.
	* ipa.c: Include target.h and tree-iterator.h.
	(cgraph_build_static_cdtor, static_ctors, static_dtors,
	record_cdtor_fn, build_cdtor, compare_ctor, compare_dtor,
	build_cdtor_fns, ipa_cdtor_merge, gate_ipa_cdtor_merge,
	pass_ipa_cdtor_merge): New.
	* passes.c (init_optimization_passes): Enqueue pass_ipa_cdtor_merge.
	* ipa-prop.c (update_indirect_edges_after_inlining): Avoid out of
	bounds access.

Index: tree-pass.h
===================================================================
--- tree-pass.h	(revision 167709)
+++ tree-pass.h	(working copy)
@@ -465,7 +465,6 @@
 extern struct ipa_opt_pass_d pass_ipa_lto_wpa_fixup;
 extern struct ipa_opt_pass_d pass_ipa_lto_finish_out;
 extern struct ipa_opt_pass_d pass_ipa_profile;
-extern struct ipa_opt_pass_d pass_ipa_cdtor_merge;
 
 extern struct gimple_opt_pass pass_all_optimizations;
 extern struct gimple_opt_pass pass_cleanup_cfg_post_optimizing;
Index: cgraphunit.c
===================================================================
--- cgraphunit.c	(revision 167709)
+++ cgraphunit.c	(working copy)
@@ -147,9 +147,174 @@
 
 FILE *cgraph_dump_file;
 
+/* A vector of FUNCTION_DECLs declared as static constructors.  */
+static GTY (()) VEC(tree, gc) *static_ctors;
+/* A vector of FUNCTION_DECLs declared as static destructors.  */
+static GTY (()) VEC(tree, gc) *static_dtors;
+
 /* Used for vtable lookup in thunk adjusting.  */
 static GTY (()) tree vtable_entry_type;
 
+/* When target does not have ctors and dtors, we call all constructor
+   and destructor by special initialization/destruction function
+   recognized by collect2.
+
+   When we are going to build this function, collect all constructors and
+   destructors and turn them into normal functions.  */
+
+static void
+record_cdtor_fn (tree fndecl)
+{
+  struct cgraph_node *node;
+  if (targetm.have_ctors_dtors
+      || (!DECL_STATIC_CONSTRUCTOR (fndecl)
+	  && !DECL_STATIC_DESTRUCTOR (fndecl)))
+    return;
+
+  if (DECL_STATIC_CONSTRUCTOR (fndecl))
+    {
+      VEC_safe_push (tree, gc, static_ctors, fndecl);
+      DECL_STATIC_CONSTRUCTOR (fndecl) = 0;
+    }
+  if (DECL_STATIC_DESTRUCTOR (fndecl))
+    {
+      VEC_safe_push (tree, gc, static_dtors, fndecl);
+      DECL_STATIC_DESTRUCTOR (fndecl) = 0;
+    }
+  node = cgraph_node (fndecl);
+  node->local.disregard_inline_limits = 1;
+  cgraph_mark_reachable_node (node);
+}
+
+/* Define global constructors/destructor functions for the CDTORS, of
+   which they are LEN.  The CDTORS are sorted by initialization
+   priority.  If CTOR_P is true, these are constructors; otherwise,
+   they are destructors.  */
+
+static void
+build_cdtor (bool ctor_p, tree *cdtors, size_t len)
+{
+  size_t i;
+
+  i = 0;
+  while (i < len)
+    {
+      tree body;
+      tree fn;
+      priority_type priority;
+
+      priority = 0;
+      body = NULL_TREE;
+      /* Find the next batch of constructors/destructors with the same
+	 initialization priority.  */
+      do
+	{
+	  priority_type p;
+	  tree call;
+	  fn = cdtors[i];
+	  p = ctor_p ? DECL_INIT_PRIORITY (fn) : DECL_FINI_PRIORITY (fn);
+	  if (!body)
+	    priority = p;
+	  else if (p != priority)
+	    break;
+	  call = build_call_expr (fn, 0);
+	  append_to_statement_list (call, &body);
+	  ++i;
+	}
+      while (i < len);
+      gcc_assert (body != NULL_TREE);
+      /* Generate a function to call all the function of like
+	 priority.  */
+      cgraph_build_static_cdtor (ctor_p ? 'I' : 'D', body, priority);
+    }
+}
+
+/* Comparison function for qsort.  P1 and P2 are actually of type
+   "tree *" and point to static constructors.  DECL_INIT_PRIORITY is
+   used to determine the sort order.  */
+
+static int
+compare_ctor (const void *p1, const void *p2)
+{
+  tree f1;
+  tree f2;
+  int priority1;
+  int priority2;
+
+  f1 = *(const tree *)p1;
+  f2 = *(const tree *)p2;
+  priority1 = DECL_INIT_PRIORITY (f1);
+  priority2 = DECL_INIT_PRIORITY (f2);
+
+  if (priority1 < priority2)
+    return -1;
+  else if (priority1 > priority2)
+    return 1;
+  else
+    /* Ensure a stable sort.  */
+    return (const tree *)p1 - (const tree *)p2;
+}
+
+/* Comparison function for qsort.  P1 and P2 are actually of type
+   "tree *" and point to static destructors.  DECL_FINI_PRIORITY is
+   used to determine the sort order.  */
+
+static int
+compare_dtor (const void *p1, const void *p2)
+{
+  tree f1;
+  tree f2;
+  int priority1;
+  int priority2;
+
+  f1 = *(const tree *)p1;
+  f2 = *(const tree *)p2;
+  priority1 = DECL_FINI_PRIORITY (f1);
+  priority2 = DECL_FINI_PRIORITY (f2);
+
+  if (priority1 < priority2)
+    return -1;
+  else if (priority1 > priority2)
+    return 1;
+  else
+    /* Ensure a stable sort.  */
+    return (const tree *)p1 - (const tree *)p2;
+}
+
+/* Generate functions to call static constructors and destructors
+   for targets that do not support .ctors/.dtors sections.  These
+   functions have magic names which are detected by collect2.  */
+
+static void
+cgraph_build_cdtor_fns (void)
+{
+  if (!VEC_empty (tree, static_ctors))
+    {
+      gcc_assert (!targetm.have_ctors_dtors);
+      qsort (VEC_address (tree, static_ctors),
+	     VEC_length (tree, static_ctors),
+	     sizeof (tree),
+	     compare_ctor);
+      build_cdtor (/*ctor_p=*/true,
+		   VEC_address (tree, static_ctors),
+		   VEC_length (tree, static_ctors));
+      VEC_truncate (tree, static_ctors, 0);
+    }
+
+  if (!VEC_empty (tree, static_dtors))
+    {
+      gcc_assert (!targetm.have_ctors_dtors);
+      qsort (VEC_address (tree, static_dtors),
+	     VEC_length (tree, static_dtors),
+	     sizeof (tree),
+	     compare_dtor);
+      build_cdtor (/*ctor_p=*/false,
+		   VEC_address (tree, static_dtors),
+		   VEC_length (tree, static_dtors));
+      VEC_truncate (tree, static_dtors, 0);
+    }
+}
+
 /* Determine if function DECL is needed.  That is, visible to something
    either outside this translation unit, something magic in the system
    configury.  */
@@ -353,6 +518,7 @@
   node->local.finalized = true;
   node->lowered = DECL_STRUCT_FUNCTION (decl)->cfg != NULL;
   node->finalized_by_frontend = true;
+  record_cdtor_fn (node->decl);
 
   if (cgraph_decide_is_function_needed (node, decl))
     cgraph_mark_needed_node (node);
@@ -1008,6 +1174,10 @@
   /* Emit size functions we didn't inline.  */
   finalize_size_functions ();
 
+  /* Call functions declared with the "constructor" or "destructor"
+     attribute.  */
+  cgraph_build_cdtor_fns ();
+
   /* Mark alias targets necessary and emit diagnostics.  */
   finish_aliases_1 ();
 
@@ -1861,7 +2031,79 @@
 #endif
 }
 
+
+/* Generate and emit a static constructor or destructor.  WHICH must
+   be one of 'I' (for a constructor) or 'D' (for a destructor).  BODY
+   is a STATEMENT_LIST containing GENERIC statements.  PRIORITY is the
+   initialization priority for this constructor or destructor.  */
+
 void
+cgraph_build_static_cdtor (char which, tree body, int priority)
+{
+  static int counter = 0;
+  char which_buf[16];
+  tree decl, name, resdecl;
+
+  /* The priority is encoded in the constructor or destructor name.
+     collect2 will sort the names and arrange that they are called at
+     program startup.  */
+  sprintf (which_buf, "%c_%.5d_%d", which, priority, counter++);
+  name = get_file_function_name (which_buf);
+
+  decl = build_decl (input_location, FUNCTION_DECL, name,
+		     build_function_type_list (void_type_node, NULL_TREE));
+  current_function_decl = decl;
+
+  resdecl = build_decl (input_location,
+			RESULT_DECL, NULL_TREE, void_type_node);
+  DECL_ARTIFICIAL (resdecl) = 1;
+  DECL_RESULT (decl) = resdecl;
+  DECL_CONTEXT (resdecl) = decl;
+
+  allocate_struct_function (decl, false);
+
+  TREE_STATIC (decl) = 1;
+  TREE_USED (decl) = 1;
+  DECL_ARTIFICIAL (decl) = 1;
+  DECL_NO_INSTRUMENT_FUNCTION_ENTRY_EXIT (decl) = 1;
+  DECL_SAVED_TREE (decl) = body;
+  if (!targetm.have_ctors_dtors)
+    {
+      TREE_PUBLIC (decl) = 1;
+      DECL_PRESERVE_P (decl) = 1;
+    }
+  DECL_UNINLINABLE (decl) = 1;
+
+  DECL_INITIAL (decl) = make_node (BLOCK);
+  TREE_USED (DECL_INITIAL (decl)) = 1;
+
+  DECL_SOURCE_LOCATION (decl) = input_location;
+  cfun->function_end_locus = input_location;
+
+  switch (which)
+    {
+    case 'I':
+      DECL_STATIC_CONSTRUCTOR (decl) = 1;
+      decl_init_priority_insert (decl, priority);
+      break;
+    case 'D':
+      DECL_STATIC_DESTRUCTOR (decl) = 1;
+      decl_fini_priority_insert (decl, priority);
+      break;
+    default:
+      gcc_unreachable ();
+    }
+
+  gimplify_function_tree (decl);
+
+  cgraph_add_new_function (decl, false);
+  cgraph_mark_needed_node (cgraph_node (decl));
+
+  set_cfun (NULL);
+  current_function_decl = NULL;
+}
+
+void
 init_cgraph (void)
 {
   if (!cgraph_dump_file)
Index: ipa.c
===================================================================
--- ipa.c	(revision 167709)
+++ ipa.c	(working copy)
@@ -29,8 +29,6 @@
 #include "ggc.h"
 #include "flags.h"
 #include "pointer-set.h"
-#include "target.h"
-#include "tree-iterator.h"
 
 /* Fill array order with all nodes with output flag set in the reverse
    topological order.  */
@@ -1492,296 +1490,3 @@
  NULL,			                /* function_transform */
  NULL					/* variable_transform */
 };
-
-/* Generate and emit a static constructor or destructor.  WHICH must
-   be one of 'I' (for a constructor) or 'D' (for a destructor).  BODY
-   is a STATEMENT_LIST containing GENERIC statements.  PRIORITY is the
-   initialization priority for this constructor or destructor.  */
-
-void
-cgraph_build_static_cdtor (char which, tree body, int priority)
-{
-  static int counter = 0;
-  char which_buf[16];
-  tree decl, name, resdecl;
-
-  /* The priority is encoded in the constructor or destructor name.
-     collect2 will sort the names and arrange that they are called at
-     program startup.  */
-  sprintf (which_buf, "%c_%.5d_%d", which, priority, counter++);
-  name = get_file_function_name (which_buf);
-
-  decl = build_decl (input_location, FUNCTION_DECL, name,
-		     build_function_type_list (void_type_node, NULL_TREE));
-  current_function_decl = decl;
-
-  resdecl = build_decl (input_location,
-			RESULT_DECL, NULL_TREE, void_type_node);
-  DECL_ARTIFICIAL (resdecl) = 1;
-  DECL_RESULT (decl) = resdecl;
-  DECL_CONTEXT (resdecl) = decl;
-
-  allocate_struct_function (decl, false);
-
-  TREE_STATIC (decl) = 1;
-  TREE_USED (decl) = 1;
-  DECL_ARTIFICIAL (decl) = 1;
-  DECL_NO_INSTRUMENT_FUNCTION_ENTRY_EXIT (decl) = 1;
-  DECL_SAVED_TREE (decl) = body;
-  if (!targetm.have_ctors_dtors)
-    {
-      TREE_PUBLIC (decl) = 1;
-      DECL_PRESERVE_P (decl) = 1;
-    }
-  DECL_UNINLINABLE (decl) = 1;
-
-  DECL_INITIAL (decl) = make_node (BLOCK);
-  TREE_USED (DECL_INITIAL (decl)) = 1;
-
-  DECL_SOURCE_LOCATION (decl) = input_location;
-  cfun->function_end_locus = input_location;
-
-  switch (which)
-    {
-    case 'I':
-      DECL_STATIC_CONSTRUCTOR (decl) = 1;
-      decl_init_priority_insert (decl, priority);
-      break;
-    case 'D':
-      DECL_STATIC_DESTRUCTOR (decl) = 1;
-      decl_fini_priority_insert (decl, priority);
-      break;
-    default:
-      gcc_unreachable ();
-    }
-
-  gimplify_function_tree (decl);
-
-  cgraph_add_new_function (decl, false);
-
-  set_cfun (NULL);
-  current_function_decl = NULL;
-}
-
-
-/* A vector of FUNCTION_DECLs declared as static constructors.  */
-static VEC(tree, heap) *static_ctors;
-/* A vector of FUNCTION_DECLs declared as static destructors.  */
-static VEC(tree, heap) *static_dtors;
-
-/* When target does not have ctors and dtors, we call all constructor
-   and destructor by special initialization/destruction function
-   recognized by collect2.
-
-   When we are going to build this function, collect all constructors and
-   destructors and turn them into normal functions.  */
-
-static void
-record_cdtor_fn (struct cgraph_node *node)
-{
-  if (DECL_STATIC_CONSTRUCTOR (node->decl))
-    VEC_safe_push (tree, heap, static_ctors, node->decl);
-  if (DECL_STATIC_DESTRUCTOR (node->decl))
-    VEC_safe_push (tree, heap, static_dtors, node->decl);
-  node = cgraph_node (node->decl);
-  node->local.disregard_inline_limits = 1;
-}
-
-/* Define global constructors/destructor functions for the CDTORS, of
-   which they are LEN.  The CDTORS are sorted by initialization
-   priority.  If CTOR_P is true, these are constructors; otherwise,
-   they are destructors.  */
-
-static void
-build_cdtor (bool ctor_p, VEC (tree, heap) *cdtors)
-{
-  size_t i,j;
-  size_t len = VEC_length (tree, cdtors);
-
-  i = 0;
-  while (i < len)
-    {
-      tree body;
-      tree fn;
-      priority_type priority;
-
-      priority = 0;
-      body = NULL_TREE;
-      j = i;
-      do
-	{
-	  priority_type p;
-	  fn = VEC_index (tree, cdtors, j);
-	  p = ctor_p ? DECL_INIT_PRIORITY (fn) : DECL_FINI_PRIORITY (fn);
-	  if (j == i)
-	    priority = p;
-	  else if (p != priority)
-	    break;
-	  j++;
-	}
-      while (j < len);
-
-      /* When there is only one cdtor and target supports them, do nothing.  */
-      if (j == i + 1
-	  && targetm.have_ctors_dtors)
-	{
-	  i++;
-	  continue;
-	}
-      /* Find the next batch of constructors/destructors with the same
-	 initialization priority.  */
-      for (;i < j; i++)
-	{
-	  tree call;
-	  fn = VEC_index (tree, cdtors, i);
-	  call = build_call_expr (fn, 0);
-	  if (ctor_p)
-	    DECL_STATIC_CONSTRUCTOR (fn) = 0;
-	  else
-	    DECL_STATIC_DESTRUCTOR (fn) = 0;
-	  /* We do not want to optimize away pure/const calls here.
-	     When optimizing, these should be already removed, when not
-	     optimizing, we want user to be able to breakpoint in them.  */
-	  TREE_SIDE_EFFECTS (call) = 1;
-	  append_to_statement_list (call, &body);
-	}
-      gcc_assert (body != NULL_TREE);
-      /* Generate a function to call all the function of like
-	 priority.  */
-      cgraph_build_static_cdtor (ctor_p ? 'I' : 'D', body, priority);
-    }
-}
-
-/* Comparison function for qsort.  P1 and P2 are actually of type
-   "tree *" and point to static constructors.  DECL_INIT_PRIORITY is
-   used to determine the sort order.  */
-
-static int
-compare_ctor (const void *p1, const void *p2)
-{
-  tree f1;
-  tree f2;
-  int priority1;
-  int priority2;
-
-  f1 = *(const tree *)p1;
-  f2 = *(const tree *)p2;
-  priority1 = DECL_INIT_PRIORITY (f1);
-  priority2 = DECL_INIT_PRIORITY (f2);
-
-  if (priority1 < priority2)
-    return -1;
-  else if (priority1 > priority2)
-    return 1;
-  else
-    /* Ensure a stable sort.  Constructors are executed in backwarding
-       order to make LTO initialize braries first.  */
-    return DECL_UID (f2) - DECL_UID (f1);
-}
-
-/* Comparison function for qsort.  P1 and P2 are actually of type
-   "tree *" and point to static destructors.  DECL_FINI_PRIORITY is
-   used to determine the sort order.  */
-
-static int
-compare_dtor (const void *p1, const void *p2)
-{
-  tree f1;
-  tree f2;
-  int priority1;
-  int priority2;
-
-  f1 = *(const tree *)p1;
-  f2 = *(const tree *)p2;
-  priority1 = DECL_FINI_PRIORITY (f1);
-  priority2 = DECL_FINI_PRIORITY (f2);
-
-  if (priority1 < priority2)
-    return -1;
-  else if (priority1 > priority2)
-    return 1;
-  else
-    /* Ensure a stable sort.  */
-    return DECL_UID (f1) - DECL_UID (f2);
-}
-
-/* Generate functions to call static constructors and destructors
-   for targets that do not support .ctors/.dtors sections.  These
-   functions have magic names which are detected by collect2.  */
-
-static void
-build_cdtor_fns (void)
-{
-  if (!VEC_empty (tree, static_ctors))
-    {
-      gcc_assert (!targetm.have_ctors_dtors || in_lto_p);
-      VEC_qsort (tree, static_ctors, compare_ctor);
-      build_cdtor (/*ctor_p=*/true, static_ctors);
-    }
-
-  if (!VEC_empty (tree, static_dtors))
-    {
-      gcc_assert (!targetm.have_ctors_dtors || in_lto_p);
-      VEC_qsort (tree, static_dtors, compare_dtor);
-      build_cdtor (/*ctor_p=*/false, static_dtors);
-    }
-}
-
-/* Look for constructors and destructors and produce function calling them.
-   This is needed for targets not supporting ctors or dtors, but we perform the
-   transformation also at linktime to merge possibly numberous
-   constructors/destructors into single function to improve code locality and
-   reduce size.  */
-
-static unsigned int
-ipa_cdtor_merge (void)
-{
-  struct cgraph_node *node;
-  for (node = cgraph_nodes; node; node = node->next)
-    if (node->analyzed
-	&& (DECL_STATIC_CONSTRUCTOR (node->decl)
-	    || DECL_STATIC_DESTRUCTOR (node->decl)))
-       record_cdtor_fn (node);
-  build_cdtor_fns ();
-  VEC_free (tree, heap, static_ctors);
-  VEC_free (tree, heap, static_dtors);
-  return 0;
-}
-
-/* Perform the pass when we have no ctors/dtors support
-   or at LTO time to merge multiple constructors into single
-   function.  */
-
-static bool
-gate_ipa_cdtor_merge (void)
-{
-  return !targetm.have_ctors_dtors || (optimize && in_lto_p);
-}
-
-struct ipa_opt_pass_d pass_ipa_cdtor_merge =
-{
- {
-  IPA_PASS,
-  "cdtor",				/* name */
-  gate_ipa_cdtor_merge,			/* gate */
-  ipa_cdtor_merge,		        /* execute */
-  NULL,					/* sub */
-  NULL,					/* next */
-  0,					/* static_pass_number */
-  TV_CGRAPHOPT,			        /* tv_id */
-  0,	                                /* properties_required */
-  0,					/* properties_provided */
-  0,					/* properties_destroyed */
-  0,					/* todo_flags_start */
-  0                                     /* todo_flags_finish */
- },
- NULL,				        /* generate_summary */
- NULL,					/* write_summary */
- NULL,					/* read_summary */
- NULL,					/* write_optimization_summary */
- NULL,					/* read_optimization_summary */
- NULL,					/* stmt_fixup */
- 0,					/* TODOs */
- NULL,			                /* function_transform */
- NULL					/* variable_transform */
-};
Index: ipa-prop.c
===================================================================
--- ipa-prop.c	(revision 167709)
+++ ipa-prop.c	(working copy)
@@ -1575,12 +1575,11 @@
 				      struct cgraph_node *node,
 				      VEC (cgraph_edge_p, heap) **new_edges)
 {
-  struct ipa_edge_args *top;
+  struct ipa_edge_args *top = IPA_EDGE_REF (cs);
   struct cgraph_edge *ie, *next_ie, *new_direct_edge;
   bool res = false;
 
   ipa_check_create_edge_args ();
-  top = IPA_EDGE_REF (cs);
 
   for (ie = node->indirect_calls; ie; ie = next_ie)
     {
Index: passes.c
===================================================================
--- passes.c	(revision 167709)
+++ passes.c	(working copy)
@@ -794,7 +794,6 @@
   NEXT_PASS (pass_ipa_whole_program_visibility);
   NEXT_PASS (pass_ipa_profile);
   NEXT_PASS (pass_ipa_cp);
-  NEXT_PASS (pass_ipa_cdtor_merge);
   NEXT_PASS (pass_ipa_inline);
   NEXT_PASS (pass_ipa_pure_const);
   NEXT_PASS (pass_ipa_reference);

Re: HPPA constructor merge patch, PR middle-end/45388

[Mark Mitchell]

On 12/12/2010 8:19 AM, John David Anglin wrote:

> It has now been nearly four months since the commit was made and
> no progress has been made in finding a fix along the lines discussed
> below.  I believe the change should be reverted to provide an
> incentive to fix the issue.

Independent of incentives, I think that the ethos in the development
community is indeed that people fix regressions.  So, wait 24 hours, and
if the regression hasn't been fixed, go ahead and check in the reversion.

Thank you,

-- 
Mark Mitchell
CodeSourcery
mark@codesourcery.com
(650) 331-3385 x713

Re: HPPA constructor merge patch, PR middle-end/45388

[Jan Hubicka]

> On 12/12/2010 8:19 AM, John David Anglin wrote:
> 
> > It has now been nearly four months since the commit was made and
> > no progress has been made in finding a fix along the lines discussed
> > below.  I believe the change should be reverted to provide an
> > incentive to fix the issue.
> 
> Independent of incentives, I think that the ethos in the development
> community is indeed that people fix regressions.  So, wait 24 hours, and
> if the regression hasn't been fixed, go ahead and check in the reversion.

Hi,
We've discussed several ways of fixing the regression.  At the moment I think
it is easiest to change DECL_ASSEMBLER_NAME to be no longer recognized by
collect2 as it is simple and localized change.  It might be better to make
collect2 to not look for static symbols but I got lost on looking into that. I
will prepare patch for htat.

Honza
> 
> Thank you,
> 
> -- 
> Mark Mitchell
> CodeSourcery
> mark@codesourcery.com
> (650) 331-3385 x713

Re: HPPA constructor merge patch, PR middle-end/45388

[Jan Hubicka]

Hi,
this patch implements actually the Richard's idea of not giving constructors
collect2 recognizable names before constructor merging pass.  

I am not aware of any setup I can test the constructors on, but it solves the
problem with covariant3 testcase.

I inspected places where cgraph_build_static_cdtor is used and I think they all
appear before constructor merging pass.  It is possible that some of them (like
profiling or emultls) do constructors late, then we will need to export the
FINAL flag out of the ipa.c and set it correspondingly.

I am testing the patch on x86_64-linux, can you, please, test it at hppa?
Sorry for the delay, I really lost track of this problem.

There is probably easier fix, the code breaks only becaue I droped the code
setting always_inline flag on the constructor generation. I did this because
one can introduce ICE by doing constructors that are actually uninlinable.
So I hope that this fix will solve both problems.

Honza

Index: cp/decl2.c
===================================================================
--- cp/decl2.c	(revision 167726)
+++ cp/decl2.c	(working copy)
@@ -2691,7 +2691,7 @@ start_objects (int method_type, int init
 {
   tree body;
   tree fndecl;
-  char type[10];
+  char type[14];
 
   /* Make ctor or dtor function.  METHOD_TYPE may be 'I' or 'D'.  */
 
@@ -2705,10 +2705,10 @@ start_objects (int method_type, int init
       joiner = '_';
 #endif
 
-      sprintf (type, "%c%c%.5u", method_type, joiner, initp);
+      sprintf (type, "sub_%c%c%.5u", method_type, joiner, initp);
     }
   else
-    sprintf (type, "%c", method_type);
+    sprintf (type, "sub_%c", method_type);
 
   fndecl = build_lang_decl (FUNCTION_DECL,
 			    get_file_function_name (type),
Index: ipa.c
===================================================================
--- ipa.c	(revision 167726)
+++ ipa.c	(working copy)
@@ -1496,10 +1496,13 @@ struct ipa_opt_pass_d pass_ipa_profile =
 /* Generate and emit a static constructor or destructor.  WHICH must
    be one of 'I' (for a constructor) or 'D' (for a destructor).  BODY
    is a STATEMENT_LIST containing GENERIC statements.  PRIORITY is the
-   initialization priority for this constructor or destructor.  */
+   initialization priority for this constructor or destructor. 
 
-void
-cgraph_build_static_cdtor (char which, tree body, int priority)
+   FINAL specify whether the externally visible name for collect2 should
+   be produced. */
+
+static void
+cgraph_build_static_cdtor_1 (char which, tree body, int priority, bool final)
 {
   static int counter = 0;
   char which_buf[16];
@@ -1508,7 +1511,12 @@ cgraph_build_static_cdtor (char which, t
   /* The priority is encoded in the constructor or destructor name.
      collect2 will sort the names and arrange that they are called at
      program startup.  */
-  sprintf (which_buf, "%c_%.5d_%d", which, priority, counter++);
+  if (final)
+    sprintf (which_buf, "%c_%.5d_%d", which, priority, counter++);
+  else
+  /* Proudce sane name but one not recognizable by collect2, just for the
+     case we fail to inline the function.  */
+    sprintf (which_buf, "sub_%c_%.5d_%d", which, priority, counter++);
   name = get_file_function_name (which_buf);
 
   decl = build_decl (input_location, FUNCTION_DECL, name,
@@ -1528,7 +1536,7 @@ cgraph_build_static_cdtor (char which, t
   DECL_ARTIFICIAL (decl) = 1;
   DECL_NO_INSTRUMENT_FUNCTION_ENTRY_EXIT (decl) = 1;
   DECL_SAVED_TREE (decl) = body;
-  if (!targetm.have_ctors_dtors)
+  if (!targetm.have_ctors_dtors && final)
     {
       TREE_PUBLIC (decl) = 1;
       DECL_PRESERVE_P (decl) = 1;
@@ -1563,6 +1571,16 @@ cgraph_build_static_cdtor (char which, t
   current_function_decl = NULL;
 }
 
+/* Generate and emit a static constructor or destructor.  WHICH must
+   be one of 'I' (for a constructor) or 'D' (for a destructor).  BODY
+   is a STATEMENT_LIST containing GENERIC statements.  PRIORITY is the
+   initialization priority for this constructor or destructor.  */
+
+void
+cgraph_build_static_cdtor (char which, tree body, int priority)
+{
+  cgraph_build_static_cdtor_1 (which, body, priority, false);
+}
 
 /* A vector of FUNCTION_DECLs declared as static constructors.  */
 static VEC(tree, heap) *static_ctors;
@@ -1648,7 +1666,7 @@ build_cdtor (bool ctor_p, VEC (tree, hea
       gcc_assert (body != NULL_TREE);
       /* Generate a function to call all the function of like
 	 priority.  */
-      cgraph_build_static_cdtor (ctor_p ? 'I' : 'D', body, priority);
+      cgraph_build_static_cdtor_1 (ctor_p ? 'I' : 'D', body, priority, true);
     }
 }
 

Re: HPPA constructor merge patch, PR middle-end/45388

[John David Anglin]

> I am testing the patch on x86_64-linux, can you, please, test it at hppa?

Will do.

Thanks,
Dave
-- 
J. David Anglin                                  dave.anglin@nrc-cnrc.gc.ca
National Research Council of Canada              (613) 990-0752 (FAX: 952-6602)

Re: HPPA constructor merge patch, PR middle-end/45388

[John David Anglin]

> 
> > I am testing the patch on x86_64-linux, can you, please, test it at hppa?
> 
> Will do.

Works.  No regressions seen in a build and check with hppa2.0w-hp-hpux11.11.

Thanks,
Dave
-- 
J. David Anglin                                  dave.anglin@nrc-cnrc.gc.ca
National Research Council of Canada              (613) 990-0752 (FAX: 952-6602)

Re: HPPA constructor merge patch, PR middle-end/45388

[Jan Hubicka]

> > 
> > > I am testing the patch on x86_64-linux, can you, please, test it at hppa?
> > 
> > Will do.
> 
> Works.  No regressions seen in a build and check with hppa2.0w-hp-hpux11.11.
Great, thanks!
Mark, does the C++ change look OK? It changes GLOBAL__I function that will never
end up being the externally visible static constructor into GLOBAL__sub_I.
We can chose any other name, but having those static constructor recognizable
name seems like not bad idea.

Bootatrapped/regtested x86_64-linux.

Honza

	PR middle-end/45388
	* decl2.c (start_objects): Do not generate collect2 recognicable name
	for static ctor.
	* ipa.c (cgraph_build_static_cdtor_1): Break out from ... ; add FINAL parameter.
	(cgraph_build_static_cdtor): ... here.
	(build_cdtor): Use cgraph_build_static_cdtor_1.
Index: cp/decl2.c
===================================================================
--- cp/decl2.c	(revision 167726)
+++ cp/decl2.c	(working copy)
@@ -2691,7 +2691,7 @@ start_objects (int method_type, int init
 {
   tree body;
   tree fndecl;
-  char type[10];
+  char type[14];
 
   /* Make ctor or dtor function.  METHOD_TYPE may be 'I' or 'D'.  */
 
@@ -2705,10 +2705,10 @@ start_objects (int method_type, int init
       joiner = '_';
 #endif
 
-      sprintf (type, "%c%c%.5u", method_type, joiner, initp);
+      sprintf (type, "sub_%c%c%.5u", method_type, joiner, initp);
     }
   else
-    sprintf (type, "%c", method_type);
+    sprintf (type, "sub_%c", method_type);
 
   fndecl = build_lang_decl (FUNCTION_DECL,
 			    get_file_function_name (type),
Index: ipa.c
===================================================================
--- ipa.c	(revision 167726)
+++ ipa.c	(working copy)
@@ -1496,10 +1496,13 @@ struct ipa_opt_pass_d pass_ipa_profile =
 /* Generate and emit a static constructor or destructor.  WHICH must
    be one of 'I' (for a constructor) or 'D' (for a destructor).  BODY
    is a STATEMENT_LIST containing GENERIC statements.  PRIORITY is the
-   initialization priority for this constructor or destructor.  */
+   initialization priority for this constructor or destructor. 
 
-void
-cgraph_build_static_cdtor (char which, tree body, int priority)
+   FINAL specify whether the externally visible name for collect2 should
+   be produced. */
+
+static void
+cgraph_build_static_cdtor_1 (char which, tree body, int priority, bool final)
 {
   static int counter = 0;
   char which_buf[16];
@@ -1508,7 +1511,12 @@ cgraph_build_static_cdtor (char which, t
   /* The priority is encoded in the constructor or destructor name.
      collect2 will sort the names and arrange that they are called at
      program startup.  */
-  sprintf (which_buf, "%c_%.5d_%d", which, priority, counter++);
+  if (final)
+    sprintf (which_buf, "%c_%.5d_%d", which, priority, counter++);
+  else
+  /* Proudce sane name but one not recognizable by collect2, just for the
+     case we fail to inline the function.  */
+    sprintf (which_buf, "sub_%c_%.5d_%d", which, priority, counter++);
   name = get_file_function_name (which_buf);
 
   decl = build_decl (input_location, FUNCTION_DECL, name,
@@ -1528,7 +1536,7 @@ cgraph_build_static_cdtor (char which, t
   DECL_ARTIFICIAL (decl) = 1;
   DECL_NO_INSTRUMENT_FUNCTION_ENTRY_EXIT (decl) = 1;
   DECL_SAVED_TREE (decl) = body;
-  if (!targetm.have_ctors_dtors)
+  if (!targetm.have_ctors_dtors && final)
     {
       TREE_PUBLIC (decl) = 1;
       DECL_PRESERVE_P (decl) = 1;
@@ -1563,6 +1571,16 @@ cgraph_build_static_cdtor (char which, t
   current_function_decl = NULL;
 }
 
+/* Generate and emit a static constructor or destructor.  WHICH must
+   be one of 'I' (for a constructor) or 'D' (for a destructor).  BODY
+   is a STATEMENT_LIST containing GENERIC statements.  PRIORITY is the
+   initialization priority for this constructor or destructor.  */
+
+void
+cgraph_build_static_cdtor (char which, tree body, int priority)
+{
+  cgraph_build_static_cdtor_1 (which, body, priority, false);
+}
 
 /* A vector of FUNCTION_DECLs declared as static constructors.  */
 static VEC(tree, heap) *static_ctors;
@@ -1648,7 +1666,7 @@ build_cdtor (bool ctor_p, VEC (tree, hea
       gcc_assert (body != NULL_TREE);
       /* Generate a function to call all the function of like
 	 priority.  */
-      cgraph_build_static_cdtor (ctor_p ? 'I' : 'D', body, priority);
+      cgraph_build_static_cdtor_1 (ctor_p ? 'I' : 'D', body, priority, true);
     }
 }
 

Re: HPPA constructor merge patch, PR middle-end/45388

[Mark Mitchell]

On 12/13/2010 5:15 AM, Jan Hubicka wrote:

> Mark, does the C++ change look OK? It changes GLOBAL__I function that will never
> end up being the externally visible static constructor into GLOBAL__sub_I.

This might break targets (if any) that still use "munch".  This is a
utility that runs nm to find static constructors and generates a C
function called __main to call them all; that allows you to implement
C++ static constructors without any linker support for things like .ctors.

I've CC'd Nathan Sidwell and Nathan Froyd here; both of them are
familiar with VxWorks, which (at least the last time I used it) still
depended on munch.  Nathans, does this still apply?

Oh, wait, Jan, is that the whole point of your patch?  That we don't
want utilities like this to recognize these things as static constructors?

Thank you,

-- 
Mark Mitchell
CodeSourcery
mark@codesourcery.com
(650) 331-3385 x713

Re: HPPA constructor merge patch, PR middle-end/45388

[Jan Hubicka]

> On 12/13/2010 5:15 AM, Jan Hubicka wrote:
> 
> > Mark, does the C++ change look OK? It changes GLOBAL__I function that will never
> > end up being the externally visible static constructor into GLOBAL__sub_I.
> 
> This might break targets (if any) that still use "munch".  This is a
> utility that runs nm to find static constructors and generates a C
> function called __main to call them all; that allows you to implement
> C++ static constructors without any linker support for things like .ctors.
> 
> I've CC'd Nathan Sidwell and Nathan Froyd here; both of them are
> familiar with VxWorks, which (at least the last time I used it) still
> depended on munch.  Nathans, does this still apply?
> 
> Oh, wait, Jan, is that the whole point of your patch?  That we don't
> want utilities like this to recognize these things as static constructors?

Yes, that is the problem.  We now produce static constructors that are later
collected into single externally visible function.  We don't want those static
ctor functions to be externally visible to collect2 or munch.
The ctor collecting for this reason was introduced couple years ago, I've now
extended it to work on ELF targets too.  While doing so, I noticed that the code
adds always_inline flag to the function that is not always safe thing to do.
Removing always inlining makes those static functions to appear in output file
and be referenced from __main.

Honza
> 
> Thank you,
> 
> -- 
> Mark Mitchell
> CodeSourcery
> mark@codesourcery.com
> (650) 331-3385 x713

Re: HPPA constructor merge patch, PR middle-end/45388

[Mark Mitchell]

On 12/13/2010 9:23 AM, Jan Hubicka wrote:

>> Oh, wait, Jan, is that the whole point of your patch?  That we don't
>> want utilities like this to recognize these things as static constructors?
> 
> Yes, that is the problem.  We now produce static constructors that are later
> collected into single externally visible function.  We don't want those static
> ctor functions to be externally visible to collect2 or munch.

OK, in that case I have no concern about changing the names.

Thank you,

-- 
Mark Mitchell
CodeSourcery
mark@codesourcery.com
(650) 331-3385 x713

Re: HPPA constructor merge patch, PR middle-end/45388

[Jan Hubicka]

> On 12/13/2010 9:23 AM, Jan Hubicka wrote:
> 
> >> Oh, wait, Jan, is that the whole point of your patch?  That we don't
> >> want utilities like this to recognize these things as static constructors?
> > 
> > Yes, that is the problem.  We now produce static constructors that are later
> > collected into single externally visible function.  We don't want those static
> > ctor functions to be externally visible to collect2 or munch.
> 
> OK, in that case I have no concern about changing the names.
Thanks,
I've commited the patch.

Honza
> 
> Thank you,
> 
> -- 
> Mark Mitchell
> CodeSourcery
> mark@codesourcery.com
> (650) 331-3385 x713

Re: HPPA constructor merge patch, PR middle-end/45388

[John David Anglin]

On Mon, 13 Dec 2010, Jan Hubicka wrote:

> I've commited the patch.

Only the ChangeLog updates were committed.

Dave
-- 
J. David Anglin                                  dave.anglin@nrc-cnrc.gc.ca
National Research Council of Canada              (613) 990-0752 (FAX: 952-6602)

Re: HPPA constructor merge patch, PR middle-end/45388

[H.J. Lu]

On Mon, Dec 13, 2010 at 9:31 AM, Jan Hubicka <hubicka@ucw.cz> wrote:
>> On 12/13/2010 9:23 AM, Jan Hubicka wrote:
>>
>> >> Oh, wait, Jan, is that the whole point of your patch?  That we don't
>> >> want utilities like this to recognize these things as static constructors?
>> >
>> > Yes, that is the problem.  We now produce static constructors that are later
>> > collected into single externally visible function.  We don't want those static
>> > ctor functions to be externally visible to collect2 or munch.
>>
>> OK, in that case I have no concern about changing the names.
> Thanks,
> I've commited the patch.
>
> Honza

This caused:

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=46933


-- 
H.J.

Re: HPPA constructor merge patch, PR middle-end/45388

[Jan Hubicka]

> This caused:
> 
> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=46933

Hi,
the problem is that gcc.dg/other/first-global testcase tests that the recognizable
name of global constructor contans name of the first global in the module.  This
is bit fragile test given the fact that ELF system don't need recognizable name here.
But since we now just add the sub_ string I guess the testcase can be updated as follows.
(the name does not change on systems that need collect2 to produce __main.  Thus the OR
sequence)

Seems to make sense?

Index: testsuite/g++.dg/other/first-global.C
===================================================================
--- testsuite/g++.dg/other/first-global.C	(revision 167752)
+++ testsuite/g++.dg/other/first-global.C	(working copy)
@@ -1,6 +1,6 @@
 /* { dg-do compile } */
 /* { dg-add-options bind_pic_locally } */
-/* { dg-final { scan-assembler "_GLOBAL__I(_|_65535_0_)foobar" } } */
+/* { dg-final { scan-assembler "_GLOBAL__(I|sub_I)(_|_65535_0_)foobar" } } */
 
 struct foo { foo (); };
 foo foobar;

Re: HPPA constructor merge patch, PR middle-end/45388

[Richard Henderson]

On 12/14/2010 01:59 AM, Jan Hubicka wrote:
> --- testsuite/g++.dg/other/first-global.C	(revision 167752)
> +++ testsuite/g++.dg/other/first-global.C	(working copy)
> @@ -1,6 +1,6 @@
>  /* { dg-do compile } */
>  /* { dg-add-options bind_pic_locally } */
> -/* { dg-final { scan-assembler "_GLOBAL__I(_|_65535_0_)foobar" } } */
> +/* { dg-final { scan-assembler "_GLOBAL__(I|sub_I)(_|_65535_0_)foobar" } } */

Ok.


r~