public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Jakub Jelinek <jakub@redhat.com>
To: "H.J. Lu" <hjl.tools@gmail.com>
Cc: GCC Patches <gcc-patches@gcc.gnu.org>
Subject: Re: PING: PATCH: PR target/46770: Use .init_array/.fini_array sections
Date: Fri, 19 Aug 2011 10:05:00 -0000	[thread overview]
Message-ID: <20110819081733.GB2687@tyan-ft48-01.lab.bos.redhat.com> (raw)
In-Reply-To: <CAMe9rOrpFYbJiNpt715qfgJO=__xxiPbjZ_E45DGB9tCefMdrQ@mail.gmail.com>

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

> --- /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.
> +
> +#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)?

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

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

> +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.).

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.

	Jakub

  parent reply	other threads:[~2011-08-19  8:17 UTC|newest]

Thread overview: 46+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-03-21 18:40 H.J. Lu
2011-03-31 15:15 ` H.J. Lu
2011-04-14 19:11   ` H.J. Lu
2011-04-26 13:08   ` H.J. Lu
2011-05-18 17:30     ` H.J. Lu
2011-06-01 23:30       ` Carrot Wei
2011-06-03  4:31       ` H.J. Lu
2011-06-03 12:31         ` Richard Guenther
2011-06-03 12:52           ` H.J. Lu
2011-06-19 20:02             ` H.J. Lu
2011-06-19 21:31               ` Uros Bizjak
2011-07-22 12:31                 ` H.J. Lu
2011-07-22 12:36                   ` Rainer Orth
2011-07-22 12:36                     ` H.J. Lu
2011-07-22 13:20                       ` Rainer Orth
2011-07-22 12:37                   ` Jakub Jelinek
2011-07-22 13:21                     ` Joseph S. Myers
2011-07-22 13:29                     ` H.J. Lu
2011-07-22 14:17                       ` H.J. Lu
2011-07-22 14:55                         ` H.J. Lu
2011-08-06 14:51                           ` H.J. Lu
2011-08-09 14:32                             ` H.J. Lu
2011-08-14 19:20                               ` H.J. Lu
2011-08-19 10:05                           ` Jakub Jelinek [this message]
2011-08-19 14:58                             ` H.J. Lu
2011-08-19 15:54                               ` Jakub Jelinek
2011-08-20 21:16                                 ` H.J. Lu
2012-03-19 20:35                                   ` DJ Delorie
2012-03-19 20:40                                     ` Andrew Pinski
2012-03-19 20:42                                       ` DJ Delorie
2011-07-01 14:12               ` H.J. Lu
2011-06-03 17:13           ` Michael Eager
2011-08-22  7:39 David Edelsohn
2011-08-22  7:44 ` H.J. Lu
2011-08-22  7:46   ` David Edelsohn
2011-08-22  7:46   ` Jakub Jelinek
2011-08-22 14:23     ` H.J. Lu
2011-08-22 15:27       ` H.J. Lu
2011-08-22 15:46         ` Paolo Bonzini
2011-08-22 17:37     ` H.J. Lu
2011-08-22 18:33       ` H.J. Lu
2011-08-22 19:12       ` Joseph S. Myers
2011-08-22 19:19         ` H.J. Lu
2011-08-22 19:38           ` Joseph S. Myers
2011-08-22 20:50             ` H.J. Lu
2011-08-22 16:26 ` H.J. Lu

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20110819081733.GB2687@tyan-ft48-01.lab.bos.redhat.com \
    --to=jakub@redhat.com \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=hjl.tools@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).