Re: PATCH: Fix the relax finalize pass

["H. J. Lu" <hjl at lucon dot org>]

On Wed, May 14, 2003 at 06:01:25PM -0700, Richard Henderson wrote:
> On Wed, May 14, 2003 at 03:53:46PM -0700, H. J. Lu wrote:
> > 	* ldlang.c (lang_size_sections_1): Take one more argument to
> > 	indicate if the relax finalize pass is needed.
> > 	(lang_size_sections): Updated.
> > 	(lang_process): Likewise.
> > 	* ldlang.h (lang_size_sections_1): Likewise.
> > 	* pe-dll.c (pe_dll_fill_sections): Likewise.
> > 	(pe_exe_fill_sections): Likewise.
> > 	* emultempl/elf32.em (gld${EMULATION_NAME}_finish): Likewise.
> > 	* emultempl/hppaelf.em (hppaelf_layout_sections_again): Likewise.
> > 	* emultempl/ppc64elf.em (ppc_before_allocation): Likewise.
> > 	(ppc_layout_sections_again): Likewise.
> > 
> > 	* ldlang.c (lang_size_sections): Don't adjust data segment
> > 	address after the relax finalize pass starts.
> > 	(lang_process): Perform the relax finalize pass only when
> > 	needed. Finalize addresses before the relax finalize pass.
> 
> Ug.  This is becoming gross.
> 
> I think the proper way to handle this is to break up the relax
> pass in bfd so that the backend gets more control.
> 
>   (1) We need a "beginning of new pass" hook that is called
>       after sizes are guessed and addresses are assigned.
>       This is where we'd assign a new GP if needed.
> 

The problem is

. = DATA_SEGMENT_ALIGN (0x10000, 0x4000);

used in elf64_ia64.xs and the corresponding code in lang_size_sections

  exp_data_seg.phase = exp_dataseg_none;
  result = lang_size_sections_1 (s, output_section_statement, prev, fill,
                                 dot, relax, check_regions);
  if (exp_data_seg.phase == exp_dataseg_end_seen)
    {
      /* If DATA_SEGMENT_ALIGN DATA_SEGMENT_END pair was seen, check whether
         a page could be saved in the data segment.  */
      bfd_vma first, last;
 
      first = -exp_data_seg.base & (exp_data_seg.pagesize - 1);
      last = exp_data_seg.end & (exp_data_seg.pagesize - 1);
      if (first && last
          && ((exp_data_seg.base & ~(exp_data_seg.pagesize - 1)) 
              != (exp_data_seg.end & ~(exp_data_seg.pagesize - 1)))
          && first + last <= exp_data_seg.pagesize)
        {
          exp_data_seg.phase = exp_dataseg_adjust;
          result = lang_size_sections_1 (s, output_section_statement, prev,
                                         fill, dot, relax, check_regions);
        }
    }

That means every time when you start a relax pass, ld will try to
squeeze one page out of the data segment. Unless you tell it not to
do it,  when the final pass starts, any section size change in the
final pass may lead to data segment address change due to the call
to lang_do_assignments after the final relax pass.


>       We'd like for this to be able to allocate a data structure
>       private to the backend, so that the pieces and passes can
>       communicate.
> 
>   (2) Existing relax hook operates on each section as usual.
>       It records information into the private data structure
>       as needed.
> 
>   (3) An "end of pass" hook that is called after all sections
>       are processed.  Using data collected from sections it
>       may decide to re-layout the GOT, or adjust dynamic 
>       relocations or whatever.  Currently we do this kind of
>       thing for every section, which wastes time.


The current code has

 if ((sec->flags & SEC_RELOC) == 0
      || sec->reloc_count == 0
      || (link_info->relax_finalizing
          && sec->need_finalize_relax == 0))
    return TRUE;

If need_finalize_relax is 0, it will return immediately.

> 
>       If this "end of pass" hook returns "no more iterations",
>       then it is also the case that the private data has been
>       deallocated.
> 
> Operation for ia64 would indeed pass through two phases, one
> in which we expand code by adding trampolines, and one in which
> we shrink data by eliminating got entries.  But all this would
> be controlled by the backend, and all lang_size_sections knows
> is that it's been told to continue around the relaxation loop.
> 
> As a side benefit, ia64 would get to keep track of the
> trampolines that it has added between sections and iterations,
> so that they can be re-used when they're in range.
> 

The only thing my current patch doesn't do is sharing trampolines.
The current linker doesn't work too well on ia64. My current patch
is an improvement. I can spend time to investigate a better solution.
But I'd like to see a working linker in the meantime.

Another simple kludge is to disable the

  if (exp_data_seg.phase == exp_dataseg_end_seen)
    {
      /* If DATA_SEGMENT_ALIGN DATA_SEGMENT_END pair was seen, check whether
         a page could be saved in the data segment.  */
	...
    }

block. We can put a FIXME there.



H.J.