This is the mail archive of the
binutils@sourceware.org
mailing list for the binutils project.
Re: PATCH: Optimize ia64 linker relaxation
- From: "H. J. Lu" <hjl at lucon dot org>
- To: James E Wilson <wilson at specifix dot com>
- Cc: binutils at sources dot redhat dot com
- Date: Wed, 5 Apr 2006 15:26:12 -0700
- Subject: Re: PATCH: Optimize ia64 linker relaxation
- References: <20060321170934.GA9506@lucon.org> <1144179801.9086.56.camel@aretha.corp.specifix.com>
On Tue, Apr 04, 2006 at 12:43:21PM -0700, James E Wilson wrote:
> On Tue, 2006-03-21 at 09:09, H. J. Lu wrote:
> > This patch optimizes ia64 linker relaxation by skipping unneeded
> > relaxation passes.
>
> It also generalizes the code to support more than the current 2
> relaxation passes, which you failed to mention. You also failed to
> mention why you want this, but we have discussed this before. You want
> to add another type of IA-64 linker relaxation which may require a 3rd
> relaxation pass.
I will look into a 3rd relaxation pass for ia64 when I find time.
>
> This looks like a reasonable cleanup, and you do have a good reason for
> wanting it, so I am willing to allow it.
>
> There is some inconsistency in how skip_relax_pass_[01] are used. Most
> places set them to TRUE/FALSE. However, one place checks the values
> against 1. It should be using TRUE instead for consistency.
>
> You added a new function relax_sections, without adding an explanatory
> comment before it.
>
> You have overloaded the relax_pass field, such that it means two
> different things, depending on where it is being used. However, the
> comment you added in bfdlink.h only documents one of the uses. This
> needs to document both of them.
Here is the updated patch.
>
> You left alone the definition of the need_finalize_relax field in the
> bfd-in2.h file, which comes from the section.c file. However, there is
> no longer any finalize relax pass, so the field name and comment don't
> make any sense. This needs to be fixed somehow. Maybe by documenting
> that the name is obsolete, or maybe by changing the name to something
> more meaningful.
There are many target specific fields in bfd_section, like
need_finalize_relax, has_gp_reloc. Right now, we have
[hjl@gnu-13 bfd]$ grep need_finalize_relax *.c | grep define
elf64-ppc.c:#define makes_toc_func_call need_finalize_relax
elfxx-ia64.c:#define skip_relax_pass_0 need_finalize_relax
I think they really should be target_bit_1, target_bit_2, ... so that
all targets can share them for their own purposes. If that is what
we want to do, I can submit a separate patch.
Thanks.
H.J.
-----
bfd/
2006-03-21 H.J. Lu <hongjiu.lu@intel.com>
* elfxx-ia64.c (elfNN_ia64_relax_section): Skip unneeded passes
with the skip_relax_pass_0 and skip_relax_pass_1 bits in the
section structure.
include/
2006-03-21 H.J. Lu <hongjiu.lu@intel.com>
* bfdlink.h (bfd_link_info): Replace need_relax_finalize with
relax_pass.
ld/
2006-03-21 H.J. Lu <hongjiu.lu@intel.com>
* emultempl/ia64elf.em: Set link_info.relax_pass to 2. Remove
link_info.need_relax_finalize.
* ldlang.c (relax_sections): New.
(lang_process): Use. Call relax_sections link_info.relax_pass
times.
* ldmain.c (main): Set link_info.relax_pass to 1. Remove
link_info.need_relax_finalize.
--- binutils/bfd/elfxx-ia64.c.relax 2006-04-05 14:28:57.000000000 -0700
+++ binutils/bfd/elfxx-ia64.c 2006-04-05 14:50:02.000000000 -0700
@@ -863,6 +863,12 @@ elfNN_ia64_relax_brl (bfd_byte *contents
bfd_putl64 (t0, hit_addr);
bfd_putl64 (t1, hit_addr + 8);
}
+
+/* Rename some of the generic section flags to better document how they
+ are used here. */
+#define skip_relax_pass_0 need_finalize_relax
+#define skip_relax_pass_1 has_gp_reloc
+
/* These functions do relaxation for IA-64 ELF. */
@@ -891,6 +897,8 @@ elfNN_ia64_relax_section (abfd, sec, lin
bfd_boolean changed_contents = FALSE;
bfd_boolean changed_relocs = FALSE;
bfd_boolean changed_got = FALSE;
+ bfd_boolean skip_relax_pass_0 = TRUE;
+ bfd_boolean skip_relax_pass_1 = TRUE;
bfd_vma gp = 0;
/* Assume we're not going to change any sizes, and we'll only need
@@ -902,11 +910,13 @@ elfNN_ia64_relax_section (abfd, sec, lin
return FALSE;
/* Nothing to do if there are no relocations or there is no need for
- the relax finalize pass. */
+ the current pass. */
if ((sec->flags & SEC_RELOC) == 0
|| sec->reloc_count == 0
- || (!link_info->need_relax_finalize
- && sec->need_finalize_relax == 0))
+ || (link_info->relax_pass == 0
+ && sec->skip_relax_pass_0 == TRUE)
+ || (link_info->relax_pass == 1
+ && sec->skip_relax_pass_1 == TRUE))
return TRUE;
symtab_hdr = &elf_tdata (abfd)->symtab_hdr;
@@ -947,20 +957,19 @@ elfNN_ia64_relax_section (abfd, sec, lin
case R_IA64_PCREL21BI:
case R_IA64_PCREL21M:
case R_IA64_PCREL21F:
- /* In the finalize pass, all br relaxations are done. We can
- skip it. */
- if (!link_info->need_relax_finalize)
+ /* In pass 1, all br relaxations are done. We can skip it. */
+ if (link_info->relax_pass == 1)
continue;
+ skip_relax_pass_0 = FALSE;
is_branch = TRUE;
break;
case R_IA64_PCREL60B:
- /* We can't optimize brl to br before the finalize pass since
- br relaxations will increase the code size. Defer it to
- the finalize pass. */
- if (link_info->need_relax_finalize)
+ /* We can't optimize brl to br in pass 0 since br relaxations
+ will increase the code size. Defer it to pass 1. */
+ if (link_info->relax_pass == 0)
{
- sec->need_finalize_relax = 1;
+ skip_relax_pass_1 = FALSE;
continue;
}
is_branch = TRUE;
@@ -968,12 +977,11 @@ elfNN_ia64_relax_section (abfd, sec, lin
case R_IA64_LTOFF22X:
case R_IA64_LDXMOV:
- /* We can't relax ldx/mov before the finalize pass since
- br relaxations will increase the code size. Defer it to
- the finalize pass. */
- if (link_info->need_relax_finalize)
+ /* We can't relax ldx/mov in pass 0 since br relaxations will
+ increase the code size. Defer it to pass 1. */
+ if (link_info->relax_pass == 0)
{
- sec->need_finalize_relax = 1;
+ skip_relax_pass_1 = FALSE;
continue;
}
is_branch = FALSE;
@@ -1363,8 +1371,12 @@ elfNN_ia64_relax_section (abfd, sec, lin
}
}
- if (!link_info->need_relax_finalize)
- sec->need_finalize_relax = 0;
+ if (link_info->relax_pass == 0)
+ {
+ /* Pass 0 is only needed to relax br. */
+ sec->skip_relax_pass_0 = skip_relax_pass_0;
+ sec->skip_relax_pass_1 = skip_relax_pass_1;
+ }
*again = changed_contents || changed_relocs;
return TRUE;
@@ -1380,6 +1392,8 @@ elfNN_ia64_relax_section (abfd, sec, lin
free (internal_relocs);
return FALSE;
}
+#undef skip_relax_pass_0
+#undef skip_relax_pass_1
static void
elfNN_ia64_relax_ldxmov (contents, off)
--- binutils/include/bfdlink.h.relax 2005-11-04 11:38:00.000000000 -0800
+++ binutils/include/bfdlink.h 2006-04-05 14:55:47.000000000 -0700
@@ -301,9 +301,6 @@ struct bfd_link_info
/* TRUE if global symbols in discarded sections should be stripped. */
unsigned int strip_discarded: 1;
- /* TRUE if the final relax pass is needed. */
- unsigned int need_relax_finalize: 1;
-
/* TRUE if generating a position independent executable. */
unsigned int pie: 1;
@@ -398,6 +395,12 @@ struct bfd_link_info
unloaded. */
const char *fini_function;
+ /* Number of relaxation passes. Usually only one relaxation pass
+ is needed. But a backend can have as many relaxation passes as
+ necessary. During bfd_relax_section call, it is set to the
+ current pass, starting from 0. */
+ int relax_pass;
+
/* Non-zero if auto-import thunks for DATA items in pei386 DLLs
should be generated/linked against. Set to 1 if this feature
is explicitly requested by the user, -1 if enabled by default. */
--- binutils/ld/emultempl/ia64elf.em.relax 2005-05-16 11:04:41.000000000 -0700
+++ binutils/ld/emultempl/ia64elf.em 2006-04-05 14:29:20.000000000 -0700
@@ -32,7 +32,7 @@ static int itanium = 0;
static void
gld${EMULATION_NAME}_after_parse (void)
{
- link_info.need_relax_finalize = TRUE;
+ link_info.relax_pass = 2;
bfd_elf${ELFSIZE}_ia64_after_parse (itanium);
}
--- binutils/ld/ldlang.c.relax 2006-04-05 11:10:55.000000000 -0700
+++ binutils/ld/ldlang.c 2006-04-05 14:54:03.000000000 -0700
@@ -5411,6 +5411,37 @@ lang_gc_sections (void)
bfd_gc_sections (output_bfd, &link_info);
}
+/* Relax all sections until bfd_relax_section gives up. */
+
+static void
+relax_sections (void)
+{
+ /* Keep relaxing until bfd_relax_section gives up. */
+ bfd_boolean relax_again;
+
+ do
+ {
+ relax_again = FALSE;
+
+ /* Note: pe-dll.c does something like this also. If you find
+ you need to change this code, you probably need to change
+ pe-dll.c also. DJ */
+
+ /* Do all the assignments with our current guesses as to
+ section sizes. */
+ lang_do_assignments ();
+
+ /* We must do this after lang_do_assignments, because it uses
+ size. */
+ lang_reset_memory_regions ();
+
+ /* Perform another relax pass - this time we know where the
+ globals are, so can make a better guess. */
+ lang_size_sections (&relax_again, FALSE);
+ }
+ while (relax_again);
+}
+
void
lang_process (void)
{
@@ -5507,38 +5538,17 @@ lang_process (void)
/* Now run around and relax if we can. */
if (command_line.relax)
{
- /* Keep relaxing until bfd_relax_section gives up. */
- bfd_boolean relax_again;
+ /* We may need more than one relaxation pass. */
+ int i = link_info.relax_pass;
- do
- {
- relax_again = FALSE;
+ /* The backend can use it to determine the current pass. */
+ link_info.relax_pass = 0;
- /* Note: pe-dll.c does something like this also. If you find
- you need to change this code, you probably need to change
- pe-dll.c also. DJ */
-
- /* Do all the assignments with our current guesses as to
- section sizes. */
- lang_do_assignments ();
-
- /* We must do this after lang_do_assignments, because it uses
- size. */
- lang_reset_memory_regions ();
-
- /* Perform another relax pass - this time we know where the
- globals are, so can make a better guess. */
- lang_size_sections (&relax_again, FALSE);
-
- /* If the normal relax is done and the relax finalize pass
- is not performed yet, we perform another relax pass. */
- if (!relax_again && link_info.need_relax_finalize)
- {
- link_info.need_relax_finalize = FALSE;
- relax_again = TRUE;
- }
+ while (i--)
+ {
+ relax_sections ();
+ link_info.relax_pass++;
}
- while (relax_again);
/* Final extra sizing to report errors. */
lang_do_assignments ();
--- binutils/ld/ldmain.c.relax 2006-03-16 12:37:43.000000000 -0800
+++ binutils/ld/ldmain.c 2006-04-05 14:29:20.000000000 -0700
@@ -313,7 +313,7 @@ main (int argc, char **argv)
link_info.spare_dynamic_tags = 5;
link_info.flags = 0;
link_info.flags_1 = 0;
- link_info.need_relax_finalize = FALSE;
+ link_info.relax_pass = 1;
link_info.warn_shared_textrel = FALSE;
link_info.gc_sections = FALSE;