* PATCH: Optimize ia64 linker relaxation
@ 2006-03-21 18:47 H. J. Lu
2006-04-04 19:43 ` James E Wilson
0 siblings, 1 reply; 6+ messages in thread
From: H. J. Lu @ 2006-03-21 18:47 UTC (permalink / raw)
To: binutils
This patch optimizes ia64 linker relaxation by skipping unneeded
relaxation passes.
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-03-16 12:37:43.000000000 -0800
+++ binutils/bfd/elfxx-ia64.c 2006-03-21 09:04:11.000000000 -0800
@@ -852,6 +852,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
+
\f
/* These functions do relaxation for IA-64 ELF. */
@@ -880,6 +886,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
@@ -891,11 +899,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 == 1)
+ || (link_info->relax_pass == 1
+ && sec->skip_relax_pass_1 == 1))
return TRUE;
symtab_hdr = &elf_tdata (abfd)->symtab_hdr;
@@ -936,20 +946,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;
@@ -957,12 +966,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;
@@ -1352,8 +1360,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;
@@ -1369,6 +1381,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-03-20 14:22:26.000000000 -0800
@@ -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,9 @@ struct bfd_link_info
unloaded. */
const char *fini_function;
+ /* Number of relaxation passes. */
+ 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-03-21 09:02:27.000000000 -0800
@@ -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-03-16 12:37:43.000000000 -0800
+++ binutils/ld/ldlang.c 2006-03-20 14:22:26.000000000 -0800
@@ -5388,6 +5388,35 @@ lang_gc_sections (void)
bfd_gc_sections (output_bfd, &link_info);
}
+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)
{
@@ -5484,38 +5513,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-03-20 14:22:26.000000000 -0800
@@ -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;
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: PATCH: Optimize ia64 linker relaxation
2006-03-21 18:47 PATCH: Optimize ia64 linker relaxation H. J. Lu
@ 2006-04-04 19:43 ` James E Wilson
2006-04-05 11:34 ` Dave Korn
2006-04-06 3:49 ` H. J. Lu
0 siblings, 2 replies; 6+ messages in thread
From: James E Wilson @ 2006-04-04 19:43 UTC (permalink / raw)
To: H. J. Lu; +Cc: binutils
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.
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.
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.
--
Jim Wilson, GNU Tools Support, http://www.specifix.com
^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: PATCH: Optimize ia64 linker relaxation
2006-04-04 19:43 ` James E Wilson
@ 2006-04-05 11:34 ` Dave Korn
2006-04-06 3:49 ` H. J. Lu
1 sibling, 0 replies; 6+ messages in thread
From: Dave Korn @ 2006-04-05 11:34 UTC (permalink / raw)
To: 'James E Wilson', 'H. J. Lu'; +Cc: binutils
On 04 April 2006 20:43, James E Wilson wrote:
> 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.
Overloaded variables make me shudder, even when they're documented; it can
all too easily become unclear which semantic is relevant/intended in which
context.
Wouldn't it be better to replace it by a two-member union and (if needed)
add a "#define relax_pass union_whatever.relax_pass" to make it more future
proof?
cheers,
DaveK
--
Can't think of a witty .sigline today....
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: PATCH: Optimize ia64 linker relaxation
2006-04-04 19:43 ` James E Wilson
2006-04-05 11:34 ` Dave Korn
@ 2006-04-06 3:49 ` H. J. Lu
2006-04-06 18:28 ` James E Wilson
1 sibling, 1 reply; 6+ messages in thread
From: H. J. Lu @ 2006-04-06 3:49 UTC (permalink / raw)
To: James E Wilson; +Cc: binutils
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
+
\f
/* 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;
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: PATCH: Optimize ia64 linker relaxation
2006-04-06 3:49 ` H. J. Lu
@ 2006-04-06 18:28 ` James E Wilson
2006-04-06 21:12 ` H. J. Lu
0 siblings, 1 reply; 6+ messages in thread
From: James E Wilson @ 2006-04-06 18:28 UTC (permalink / raw)
To: H. J. Lu; +Cc: binutils
On Wed, 2006-04-05 at 15:26, H. J. Lu wrote:
> There are many target specific fields in bfd_section, like
> need_finalize_relax, has_gp_reloc. Right now, we have
I see that has_gp_reloc has the same problem. It is used in only one
place, in elf64-ppc.c, and that file uses a define to rename it to
has_toc_reloc.
I think renaming them to something like target_bit_1 is a good idea. It
solves the problem of trying to remember why we have need_finalize_relax
and has_gp_reloc fields in bfd_section.
> + /* We may need more than one relaxation pass. */
> + int i = link_info.relax_pass;
> + /* The backend can use it to determine the current pass. */
> + link_info.relax_pass = 0;
> + while (i--)
> + {
> + relax_sections ();
> + link_info.relax_pass++;
> }
Dave Korn complained about the dual meaning of relax_pass. One way to
solve it would be to rewrite this code as
while (link_info.relax_pass--)
relax_sections ();
And now relax_pass has only one meaning: the number of remaining
relaxation passes. It can still be used by the backends to determine
the current pass, they just need to count backwards instead of
forwards. It isn't clear whether this is better though, since the
current dual meaning doesn't seem that confusing to me, especially now
that it is documented.
The patch looks OK to me.
--
Jim Wilson, GNU Tools Support, http://www.specifix.com
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: PATCH: Optimize ia64 linker relaxation
2006-04-06 18:28 ` James E Wilson
@ 2006-04-06 21:12 ` H. J. Lu
0 siblings, 0 replies; 6+ messages in thread
From: H. J. Lu @ 2006-04-06 21:12 UTC (permalink / raw)
To: James E Wilson; +Cc: binutils
On Thu, Apr 06, 2006 at 11:16:27AM -0700, James E Wilson wrote:
> On Wed, 2006-04-05 at 15:26, H. J. Lu wrote:
> > There are many target specific fields in bfd_section, like
> > need_finalize_relax, has_gp_reloc. Right now, we have
>
> I see that has_gp_reloc has the same problem. It is used in only one
> place, in elf64-ppc.c, and that file uses a define to rename it to
> has_toc_reloc.
>
> I think renaming them to something like target_bit_1 is a good idea. It
> solves the problem of trying to remember why we have need_finalize_relax
> and has_gp_reloc fields in bfd_section.
I will submit a patch to rename those bits to sec_target_bit_N.
>
>
> The patch looks OK to me.
This is the patch I checked in. I used
+ || (link_info->relax_pass == 0 && sec->skip_relax_pass_0)
+ || (link_info->relax_pass == 1 && sec->skip_relax_pass_1))
instead of sec->skip_relax_pass_0 == TRUE.
Thanks.
H.J.
---
bfd/
2006-04-06 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-04-06 H.J. Lu <hongjiu.lu@intel.com>
* bfdlink.h (bfd_link_info): Replace need_relax_finalize with
relax_pass.
ld/
2006-04-06 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-06 06:55:28.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
+
\f
/* 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,11 @@ 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)
+ || (link_info->relax_pass == 1 && sec->skip_relax_pass_1))
return TRUE;
symtab_hdr = &elf_tdata (abfd)->symtab_hdr;
@@ -947,20 +955,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 +975,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 +1369,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 +1390,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;
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2006-04-06 18:56 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2006-03-21 18:47 PATCH: Optimize ia64 linker relaxation H. J. Lu
2006-04-04 19:43 ` James E Wilson
2006-04-05 11:34 ` Dave Korn
2006-04-06 3:49 ` H. J. Lu
2006-04-06 18:28 ` James E Wilson
2006-04-06 21:12 ` H. J. Lu
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).