Rework MIPS macro relaxation, fix string merging bug

[Richard Sandiford <rsandifo at redhat dot com>]

This is a roundabout fix for the string merging problem that I so lamely
thought was a gcc bug.  As Jakub pointed out, it's really a gas bug.

The problem is that the assembler macro in:

	.abicalls
	la	$4,L2 + 2
        .section        .rodata.str1.1,"aMS",@progbits,1
L1:     .string "foo"
L2:     .string "a"

will produce relocations against .rodata.str1.1, not L2.  This is
because of the way the mips relaxation machinery works.  We generate
two versions of the macro, one for when L2 is global and one for
when it is local.  However, we only emit fixups for the global case
(which is basically the default choice).  Relocs for the local case
are generated by tc_gen_reloc based on the global fixups.

Unfortunately, the fixup for the global case will have an offset of
zero, which means the target-independent code can quite legitimately
convert it into section-relative form.

For example, suppose we're generating o32 code.  The global
alternative is:

	lw $4,%got(L2)($gp)
	nop
	addiu $4,$4,2

By the time tc_gen_reloc gets to see this sequence, the target-independent
code will have changed it to:

	lw $4,%got(.rodata.str1.1 + 4)($gp)
	nop
	addiu $4,$4,2

Since the fixup is against a local symbol, t_g_r will generate two
relocs: one R_MIPS_GOT16, which replaces the original global reloc,
and one R_MIPS_LO16, which applies to the addiu.  The "addend" field
of both is 4, but is cumulative with the addiu's "2", which has become
an in-place addend for the LO16.  Thus (taking into account the link
between R_MIPS_GOT16 and R_MIPS_LO16 relocations), the final form will be:

	lw $4,%got(.rodata.str1.1 + 6)($gp)
	nop
	addiu $4,$4,%lo(.rodata.str1.1 + 6)

whereas we really need "L2 + 2".  One way of fixing this would be
to copy gas:write.c's:

	/* Never adjust a reloc against local symbol in a merge section
	   with non-zero addend.  */
	if ((symsec->flags & SEC_MERGE) != 0
	    && (fixp->fx_offset != 0 || fixp->fx_subsy != NULL))
	  continue;

to tc_fix_adjustable and extend it to cope with hidden addends.
But this seems like a bad idea on general principle, and is quite
hard to do anyway.

Instead, I'd like to remove the relaxation code from tc_gen_reloc
and generate fixups for both versions of a relaxable macro.

To recap, relaxable macros are currently built with:

	frag_grow (maximum size of both alternatives combined)
	...generate first sequence...
	p = frag_var (...complex RELAX_ENCODE invocation...)
	...generate second sequence at *p ...

Sometimes the second sequence is a complete replacement for the first.
Sometimes it adds or modifies fixups for the first sequence, perhaps
adding some more instructions of its own.  No fixup structures are
created while building the second sequence.

I'd like to change this to:

	relax_start (controlling symbol)
	...generate first sequence...
	relax_switch ();
	...generate second sequence (in full)...
	relax_end ();

where both sequences produce fixups.  As well as fixing the bug,
it has several other advantages:

  - It removes the need for the "place" argument to macro_build(),
    simplifying the code a little.

  - It makes it easier to do ad-hoc variations, such as removing
    load-delay nops for mips2 and above.

  - It makes it possible to relax n64 NO_PIC sequences into GP-relative
    sequences.

However, the patch doesn't try to do any of these.  In particular,
it keeps the macro_build() argument, even though its value is no
longer used.  I thought this was better than drowning the main change
(which is quite finicky) in a sea of other stuff.  I'll try to clean
things up in a follow-on patch.

A couple of other things I'd like to do in follow-on patches (but I'm
making no promises ;):

  - Remove the counter argument from macro_build() and generally improve
    the way we warn about multi-instruction macros.

  - Reduce the amount of cut-&-paste code, especially wrt addressing.

Anyway, back to the change in hand.  It needs the relaxation code to:

  (1) cope with cases where one relaxation is spread over several frags
      (all of which will be variant frags).

  (2) disable fixups for the case that isn't chosen.

  (3) adjust the addresses of fixups for the case that is chosen.

Which sounds bad, but turns out to be fairly easy.  I hope the
comments in the code explain what's going on.

Note that I've added two new helper functions, load_got_offset
and add_got_offset.  They are just there to prevent gratuitous
code duplication: they might not be needed if the various copies
of the address-handling code are later combined.

Also, I thought it was better to enforce a strict relax_start->
relax_switch->relax_end sequence than to allow things like a
relax_end without a relax_start.  Some macros therefore need:

	  if (mips_relax.sequence)
	    relax_end ();

although again, this is something might go away after further
reorganisation.

Patch tested by bootstrapping and regtesting gcc on:

   mips64-linux-gnu, testing {,-mabi=32,-mabi=64}
   mips64el-linux-gnu, testing {,-mabi=32,-mabi=64}{-mxgot}

Also tested on mips64vrel-elf, which, with all its multlibs, should
cover all the NO_PIC cases.  Tested against the binutils testsuite
on mips64{,el}-linux-gnu and mips64{,el}-elf.  OK to install?

Richard

[Diffed with -b because there are some indentation changes.]


gas/
	* frags.h (frag_room): Declare.
	* frags.c (frag_room): New function.
	* config/tc-mips.h (tc_frag_data_type, TC_FRAG_TYPE): Remove.
	* config/tc-mips.c (RELAX_ENCODE): Take three arguments: the size of
	the first sequence, the size of the second sequence, and a flag
	that says whether we should warn.
	(RELAX_OLD, RELAX_NEW, RELAX_RELOC[123]): Delete.
	(RELAX_FIRST, RELAX_SECOND): New.
	(mips_relax): New variable.
	(relax_close_frag, relax_start, relax_switch, relax_end): New fns.
	(append_insn): Remove "place" argument.  Use mips_relax.sequence
	rather than "place" to check whether we're expanding the second
	alternative of a relaxable macro.  Remove redundant check for
	branch relaxation.  If generating a normal insn, and there
	is not enough room in the current frag, call relax_close_frag()
	to close it.  Update mips_relax.sizes[].  Emit fixups for the
	second version of a relaxable macro.  Record the first relaxable
	fixup in mips_relax.  Remove tc_gen_reloc workaround.
	(macro_build): Remove all uses of "place".  Use mips_relax.sequence
	in the same way as in append_insn.
	(mips16_macro_build): Remove "place" argument.
	(macro_build_lui): As for macro_build.  Don't drop the add_symbol
	when generating the second version of a relaxable macro.
	(load_got_offset, add_got_offset): New functions.
	(load_address, macro): Use new relaxation machinery.  Remove
	tc_gen_reloc workarounds.
	(md_estimate_size_before_relax): Set RELAX_USE_SECOND if the second
	version of a relaxable macro is needed.  Return -RELAX_SECOND if the
	first version is needed.
	(tc_gen_reloc): Remove relaxation handling.
	(md_convert_frag): Go through the fixups for a relaxable macro and
	mark those that belong to the unneeded alternative as done.  If the
	second alternative is needed, adjust the fixup addresses to account
	for the deleted first alternative.

testsuite/
	* gas/mips/elf-rel19.[sd]: New test.
	* gas/mips/mips.exp: Run it.

Attachment: relax.diff.bz2
Description: Binary data