Re: PING: PATCH: PR target/46770: Use .init_array/.fini_array sections

["H.J. Lu" <hjl.tools@gmail.com>]

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

On Fri, Aug 19, 2011 at 1:17 AM, Jakub Jelinek <jakub@redhat.com> wrote:
> Sorry for the delay.
>
>> --- a/gcc/config.gcc
>> +++ b/gcc/config.gcc
>> @@ -186,6 +186,9 @@
>>  #  configure_default_options
>>  #                    Set to an initializer for configure_default_options
>>  #                    in configargs.h, based on --with-cpu et cetera.
>> +#
>> +#  use_initfini_array        If set to yes, .init_array/.fini_array sections
>> +#                    will be used if they work.
>>
>>  # The following variables are used in each case-construct to build up the
>>  # outgoing variables:
>> @@ -238,6 +241,7 @@ default_gnu_indirect_function=no
>>  target_gtfiles=
>>  need_64bit_hwint=
>>  need_64bit_isa=
>> +use_initfini_array=yes
>
> What is this for, when nothing ever sets it to anything but yes?
> If the $enable_initfini_array = yes test works, then there shouldn't be
> any need to override it on a per-target basis...

Done.

>> --- /dev/null
>> +++ b/gcc/config/initfini-array.h
>> @@ -0,0 +1,44 @@
>> +/* Definitions for ELF systems with .init_array/.fini_array section
>> +   support.
>> +   Copyright (C) 2011
>> +   Free Software Foundation, Inc.
>> +
>> +   This file is part of GCC.
>> +
>> +   GCC is free software; you can redistribute it and/or modify it
>> +   under the terms of the GNU General Public License as published
>> +   by the Free Software Foundation; either version 3, or (at your
>> +   option) any later version.
>> +
>> +   GCC is distributed in the hope that it will be useful, but WITHOUT
>> +   ANY WARRANTY; without even the implied warranty of MERCHANTABILITY
>> +   or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public
>> +   License for more details.
>> +
>> +   You should have received a copy of the GNU General Public License
>> +   along with GCC; see the file COPYING3.  If not see
>> +   <http://www.gnu.org/licenses/>.  */
>> +
>> +#define USE_INITFINI_ARRAY
>> +
>> +#undef INIT_SECTION_ASM_OP
>> +#undef FINI_SECTION_ASM_OP
>> +
>> +/* FIXME: INIT_ARRAY_SECTION_ASM_OP and FINI_ARRAY_SECTION_ASM_OP
>> +       aren't used in any assembly codes.  But we have to define
>> +       them to something.  */
>> +#define INIT_ARRAY_SECTION_ASM_OP Something
>> +#define FINI_ARRAY_SECTION_ASM_OP Something
>
> Can't you just define it to an empty string?  And, a couple of targets
> define INIT_ARRAY_SECTION_ASM_OP/FINI_ARRAY_SECTION_ASM_OP, you either need
> to undef it first, or define only if it wasn't defined.

Done.

>> +
>> +#ifndef TARGET_ASM_INIT_SECTIONS
>> +#define TARGET_ASM_INIT_SECTIONS default_elf_initfini_array_init_sections
>> +#endif
>> +extern void default_elf_initfini_array_init_sections (void);
>
> Why do you need this (and the default_initfini_array_init_sections () call
> in all the backends)?  Isn't it easier to just initialize the two global
> vars only when you are actually going to use them (if they are still NULL)?

Done.

>> --- a/gcc/varasm.c
>> +++ b/gcc/varasm.c
>> @@ -7350,4 +7350,62 @@ make_debug_expr_from_rtl (const_rtx exp)
>>    return dval;
>>  }
>>
>> +static GTY(()) section *elf_init_array_section;
>> +static GTY(()) section *elf_fini_array_section;
>> +
>> +void
>> +default_elf_initfini_array_init_sections (void)
>> +{
>> +  elf_init_array_section = get_unnamed_section (0, output_section_asm_op,
>> +                                             "\t.section\t.init_array");
>> +  elf_fini_array_section = get_unnamed_section (0, output_section_asm_op,
>> +                                             "\t.section\t.fini_array");
>> +}
>
> Remove above function.

Done.

>> +
>> +static section *
>> +get_elf_initfini_array_priority_section (int priority,
>> +                                      bool constructor_p)
>> +{
>> +  section *sec;
>> +  if (priority != DEFAULT_INIT_PRIORITY)
>> +    {
>> +      char buf[18];
>> +      sprintf (buf, "%s.%.5u",
>> +            constructor_p ? ".init_array" : ".fini_array",
>> +            priority);
>> +      sec = get_section (buf, SECTION_WRITE, NULL_TREE);
>> +    }
>
> I'd just put here
>   else
>     {
>       if (elf_init_array_section == NULL)
>         elf_init_array_section = get_unnamed_section...
>       if (elf_fini_array_section == NULL)
>         elf_fini_array_section = get_unnamed_section...
>> +    sec = constructor_p ? elf_init_array_section : elf_fini_array_section;
>     }

Done.

>> +void
>> +default_initfini_array_init_sections (void)
>> +{
>> +#ifdef USE_INITFINI_ARRAY
>> +  default_elf_initfini_array_init_sections ();
>> +#endif
>> +}
>
> And remove this (and all callers etc.).

Done.

> On which targets has it been tested?  Would be nice to test it at least on
> targets that define their own INIT_ARRAY_SECTION_ASM_OP (pa64-hpux, arm,
> m32c, rx) and on {i?86,x86_64,ia64}-linux and some solaris target.
>

Here is the updated patch. I tested it on Linux/ia32 and Linux/x86-64.
OK for trunk?

Bootstrap on Linux/ia64 has failed for a while and I don't have other
platforms. If it can't be easily tested on other affected platforms, can
we just enable it for Linux/ia32 and Linux/x86-64 first?

Thanks.

-- 
H.J.
----
2011-08-19  H.J. Lu  <hongjiu.lu@intel.com>

	PR target/46770
	* config.gcc (tm_file): Add initfini-array.h if
	.init_arary/.fini_array supported.

	* crtstuff.c: Don't generate .ctors nor .dtors sections if
	USE_INITFINI_ARRAY is defined.

	* varasm.c (elf_init_array_section): New.
	(elf_fini_array_section): Likewise.
	(get_elf_initfini_array_priority_section): Likewise.
	(default_elf_init_array_asm_out_constructor): Likewise.
	(default_elf_fini_array_asm_out_destructor): Likewise.

	* config/initfini-array.h: New.

[-- Attachment #2: gcc-pr46770-9.patch --]
[-- Type: text/plain, Size: 6499 bytes --]

2011-08-19  H.J. Lu  <hongjiu.lu@intel.com>

	PR target/46770
	* config.gcc (tm_file): Add initfini-array.h if
	.init_arary/.fini_array supported.

	* crtstuff.c: Don't generate .ctors nor .dtors sections if
	USE_INITFINI_ARRAY is defined.

	* varasm.c (elf_init_array_section): New.
	(elf_fini_array_section): Likewise.
	(get_elf_initfini_array_priority_section): Likewise.
	(default_elf_init_array_asm_out_constructor): Likewise.
	(default_elf_fini_array_asm_out_destructor): Likewise.

	* config/initfini-array.h: New.

diff --git a/gcc/config.gcc b/gcc/config.gcc
index b92ce3d..7f29213 100644
--- a/gcc/config.gcc
+++ b/gcc/config.gcc
@@ -3058,6 +3058,11 @@ if test x$with_schedule = x; then
 	esac
 fi
 
+# Support --enable-initfini-array.
+if test x$enable_initfini_array = xyes; then
+  tm_file="${tm_file} initfini-array.h"
+fi
+
 # Validate and mark as valid any --with options supported
 # by this target.  In order to use a particular --with option
 # you must list it in supported_defaults; validating the value
diff --git a/gcc/config/initfini-array.h b/gcc/config/initfini-array.h
new file mode 100644
index 0000000..b5b95cb
--- /dev/null
+++ b/gcc/config/initfini-array.h
@@ -0,0 +1,39 @@
+/* Definitions for ELF systems with .init_array/.fini_array section
+   support.
+   Copyright (C) 2011
+   Free Software Foundation, Inc.
+
+   This file is part of GCC.
+
+   GCC is free software; you can redistribute it and/or modify it
+   under the terms of the GNU General Public License as published
+   by the Free Software Foundation; either version 3, or (at your
+   option) any later version.
+
+   GCC is distributed in the hope that it will be useful, but WITHOUT
+   ANY WARRANTY; without even the implied warranty of MERCHANTABILITY
+   or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public
+   License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with GCC; see the file COPYING3.  If not see
+   <http://www.gnu.org/licenses/>.  */
+
+#define USE_INITFINI_ARRAY
+
+#undef INIT_SECTION_ASM_OP
+#undef FINI_SECTION_ASM_OP
+
+#undef INIT_ARRAY_SECTION_ASM_OP
+#define INIT_ARRAY_SECTION_ASM_OP
+
+#undef FINI_ARRAY_SECTION_ASM_OP
+#define FINI_ARRAY_SECTION_ASM_OP
+
+/* Use .init_array/.fini_array section for constructors and destructors. */
+#undef TARGET_ASM_CONSTRUCTOR
+#define TARGET_ASM_CONSTRUCTOR default_elf_init_array_asm_out_constructor
+#undef TARGET_ASM_DESTRUCTOR
+#define TARGET_ASM_DESTRUCTOR default_elf_fini_array_asm_out_destructor
+extern void default_elf_init_array_asm_out_constructor (rtx, int);
+extern void default_elf_fini_array_asm_out_destructor (rtx, int);
diff --git a/gcc/crtstuff.c b/gcc/crtstuff.c
index b65f490..010d472 100644
--- a/gcc/crtstuff.c
+++ b/gcc/crtstuff.c
@@ -1,7 +1,8 @@
 /* Specialized bits of code needed to support construction and
    destruction of file-scope objects in C++ code.
    Copyright (C) 1991, 1994, 1995, 1996, 1997, 1998, 1999, 2000, 2001
-   2002, 2003, 2004, 2005, 2006, 2007, 2009, 2010 Free Software Foundation, Inc.
+   2002, 2003, 2004, 2005, 2006, 2007, 2009, 2010, 2011
+   Free Software Foundation, Inc.
    Contributed by Ron Guilmette (rfg@monkeys.com).
 
 This file is part of GCC.
@@ -189,6 +190,9 @@ typedef void (*func_ptr) (void);
    refer to only the __CTOR_END__ symbol in crtend.o and the __DTOR_LIST__
    symbol in crtbegin.o, where they are defined.  */
 
+/* No need for .ctors/.dtors section if linker can place them in
+   .init_array/.fini_array section.  */
+#ifndef USE_INITFINI_ARRAY
 /* The -1 is a flag to __do_global_[cd]tors indicating that this table
    does not start with a count of elements.  */
 #ifdef CTOR_LIST_BEGIN
@@ -219,6 +223,7 @@ STATIC func_ptr __DTOR_LIST__[1]
   __attribute__((section(".dtors"), aligned(sizeof(func_ptr))))
   = { (func_ptr) (-1) };
 #endif /* __DTOR_LIST__ alternatives */
+#endif /* USE_INITFINI_ARRAY */
 
 #ifdef USE_EH_FRAME_REGISTRY
 /* Stick a label at the beginning of the frame unwind info so we can register
@@ -489,6 +494,9 @@ __do_global_ctors_1(void)
 
 #elif defined(CRT_END) /* ! CRT_BEGIN */
 
+/* No need for .ctors/.dtors section if linker can place them in
+   .init_array/.fini_array section.  */
+#ifndef USE_INITFINI_ARRAY
 /* Put a word containing zero at the end of each of our two lists of function
    addresses.  Note that the words defined here go into the .ctors and .dtors
    sections of the crtend.o file, and since that file is always linked in
@@ -534,6 +542,7 @@ STATIC func_ptr __DTOR_END__[1]
   __attribute__((used, section(".dtors"), aligned(sizeof(func_ptr))))
   = { (func_ptr) 0 };
 #endif
+#endif /* USE_INITFINI_ARRAY */
 
 #ifdef EH_FRAME_SECTION_NAME
 /* Terminate the frame unwind info section with a 4byte 0 as a sentinel;
diff --git a/gcc/varasm.c b/gcc/varasm.c
index ca56813..218baf3 100644
--- a/gcc/varasm.c
+++ b/gcc/varasm.c
@@ -7352,4 +7352,62 @@ make_debug_expr_from_rtl (const_rtx exp)
   return dval;
 }
 
+static GTY(()) section *elf_init_array_section;
+static GTY(()) section *elf_fini_array_section;
+
+static section *
+get_elf_initfini_array_priority_section (int priority,
+					 bool constructor_p)
+{
+  section *sec;
+  if (priority != DEFAULT_INIT_PRIORITY)
+    {
+      char buf[18];
+      sprintf (buf, "%s.%.5u", 
+	       constructor_p ? ".init_array" : ".fini_array",
+	       priority);
+      sec = get_section (buf, SECTION_WRITE, NULL_TREE);
+    }
+  else
+    {
+      if (constructor_p)
+	{
+	  if (elf_init_array_section == NULL)
+	    elf_init_array_section
+	      = get_unnamed_section (0, output_section_asm_op,
+				     "\t.section\t.init_array");
+	  sec = elf_init_array_section;
+	}
+      else
+	{
+	  if (elf_fini_array_section == NULL)
+	    elf_fini_array_section
+	      = get_unnamed_section (0, output_section_asm_op,
+				     "\t.section\t.fini_array");
+	  sec = elf_fini_array_section;
+	}
+    }
+  return sec;
+}
+
+/* Use .init_array section for constructors. */
+
+void
+default_elf_init_array_asm_out_constructor (rtx symbol, int priority)
+{
+  section *sec = get_elf_initfini_array_priority_section (priority,
+							  true);
+  assemble_addr_to_section (symbol, sec);
+}
+
+/* Use .fini_array section for destructors. */
+
+void
+default_elf_fini_array_asm_out_destructor (rtx symbol, int priority)
+{
+  section *sec = get_elf_initfini_array_priority_section (priority,
+							  false);
+  assemble_addr_to_section (symbol, sec);
+}
+
 #include "gt-varasm.h"