[PATCH 06/15] MIPS/GAS/test: Adjust LD for multi-target testing

[PATCH 06/15] MIPS/GAS/test: Adjust LD for multi-target testing

[Maciej W. Rozycki]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 1441 bytes --]

Hi,

 Here is a set of changes to make the LD test cases more suitable for 
multi-target testing; in particular for ELF and ECOFF targets that these 
cases are run for.  Apply some clean-ups too

 Here is a list of changes:

1. Section offsets and addends are spelled out explicitly so as to catch 
   any unexpected changes.

2. Consequently section alignment is requested at 4k so that offsets are 
   round and do not change for ECOFF when instructions are added or 
   removed (ECOFF seems to make data relocations relative to .text).

3. Missing "$" prefixes are added to the names of FP registers (shows how 
   frequently some of the targets are tested, hmm...).

4. Padding is added at the end of .text so that alignment does not matter.

5. Some minor inconsistencies are removed, e.g. the .text thing mentioned 
   previously, escapes added to "." characters that are meant to match 
   verbatim.

6. Probably something else; it's all in the patch. ;)

2010-10-03  Maciej W. Rozycki  <macro@linux-mips.org>

	gas/testsuite/
	* gas/mips/ld.d: Spell out section offsets and addends
	explicitly.  Clean up some regexps.
	* gas/mips/ld-ilocks.d: Likewise.  Add missing "$" prefixes to
	the names of FP registers.
	* gas/mips/ld-ilocks-addr32.d: Likewise.
	* gas/mips/ld.s: Align sections to 4k, adjust padding.

 OK to apply?

  Maciej

binutils-2.20.51-20100925-mips-gas-test-ld-misc.patch
[Patch attached compressed due to its size.]

[-- Attachment #2: Type: APPLICATION/X-BZIP2, Size: 2824 bytes --]

Re: [PATCH 06/15] MIPS/GAS/test: Adjust LD for multi-target testing

[Richard Sandiford]

"Maciej W. Rozycki" <macro@linux-mips.org> writes:
> 2010-10-03  Maciej W. Rozycki  <macro@linux-mips.org>
>
> 	gas/testsuite/
> 	* gas/mips/ld.d: Spell out section offsets and addends
> 	explicitly.  Clean up some regexps.
> 	* gas/mips/ld-ilocks.d: Likewise.  Add missing "$" prefixes to
> 	the names of FP registers.
> 	* gas/mips/ld-ilocks-addr32.d: Likewise.
> 	* gas/mips/ld.s: Align sections to 4k, adjust padding.

The patch seems to contain some parts that make the matching more strict
and some that make it less so.  The former look good, but I'm less sure
about the latter.  For example, with something like:

[0-9a-f]+ <[^>]*> lw   a0,(0|4096)\(at\)
[      ]*[0-9a-f]+: [A-Z0-9_]*LO[A-Z0-9_]*     \.data(\+0xfffff000)?

the difference seems relatively important: on some targets the addend
must be in-place, on some it mustn't be.  It might make more sense
to have separate dumps for the two cases.  However, that sort of
difference is probably picked up by other tests too, so the risk
might not be great.

Also, how confident are we that the more relaxed tests are correct
for ECOFF?  You say:

> 2. Consequently section alignment is requested at 4k so that offsets are 
>    round and do not change for ECOFF when instructions are added or 
>    removed (ECOFF seems to make data relocations relative to .text).

but is that intentional?  The point here, and here:

> 3. Missing "$" prefixes are added to the names of FP registers (shows how 
>    frequently some of the targets are tested, hmm...).

is precisely that mips*-ecoff has severely bitrotted from lack of
active interest, so who knows how good the current output actually is?
There's a danger in simply making the testsuite match what the current
ECOFF assembler does.

In a similar vein, the difference between endiannesses is something
that could easily go wrong, but there are separate tests that should
pick that up.  I'm much more comfortable with the endianness bits.

Overall the patch is improvement, so it's OK to apply despite
the concerns above.

Richard

Re: [PATCH 06/15] MIPS/GAS/test: Adjust LD for multi-target testing

[Maciej W. Rozycki]

On Sun, 10 Oct 2010, Richard Sandiford wrote:

> The patch seems to contain some parts that make the matching more strict
> and some that make it less so.  The former look good, but I'm less sure
> about the latter.  For example, with something like:
> 
> [0-9a-f]+ <[^>]*> lw   a0,(0|4096)\(at\)
> [      ]*[0-9a-f]+: [A-Z0-9_]*LO[A-Z0-9_]*     \.data(\+0xfffff000)?
> 
> the difference seems relatively important: on some targets the addend
> must be in-place, on some it mustn't be.  It might make more sense
> to have separate dumps for the two cases.  However, that sort of
> difference is probably picked up by other tests too, so the risk
> might not be great.

 This is certainly strictier than the original that for the record was:

[0-9a-f]+ <[^>]*> lw	a0,0\(at\)
[ 	]*[0-9a-f]+: [A-Z0-9_]*LO[A-Z0-9_]*	.data.*

(and obviously failed for ECOFF because of the different in-place addend).

 I have actually weighed the strictness of the patterns used against 
long-time maintenance effort and I think my proposal is a reasonable 
compromise.  The probability of the addend (the in-place one and/or one in 
the relocation) being wrong exactly such that it would still match all the 
patterns is minimal (which, under the appropriate Murphy's law, is 
guaranteed to happen, I know; or does it only apply to the impossible? ;) 
).  Having a separate dump for ECOFF is OTOH guaranteed to leave it in 
neglection since the moment I will have committed it.  Having the dumps 
combined gives a good chance the ECOFF variant of the testcases involved 
will stay stay either in the working order or close to for a while.

 As observed with some other changes made in this series, ECOFF testsuite 
failures are mainly if not exclusively caused by bitrot in the testsuite 
itself rather than from actual code of binutils malfunctioning.  
Regrettably I found no way to avoid creating a couple of ECOFF-specific 
dumps in this series, but for the reason stated I'd like to avoid them as 
much as possible.

 As a side note, I'd be more than happy to come up with a solution that 
could let us make exact matches of addresses dumped without sacrificing 
the ability to insert or delete pieces of test cases while keeping the 
diffs applied to dumps readable.  What I have in mind is an expression 
that would match an address printed on the previous (or an earlier) line, 
possibly offset by a constant specified in the dump.  That would require 
changes to the test framework more substantial than a small adjustment, so 
while I'm keeping this enhancement in my mind, I don't feel like I have 
resources available at hand (time, in particular) to dig into it.

> Also, how confident are we that the more relaxed tests are correct
> for ECOFF?  You say:
> 
> > 2. Consequently section alignment is requested at 4k so that offsets are 
> >    round and do not change for ECOFF when instructions are added or 
> >    removed (ECOFF seems to make data relocations relative to .text).
> 
> but is that intentional?

 I haven't investigated the way ECOFF relocations work and my knowledge of 
the executable format consists of the notion that it exists.  Someone has 
added these trailing .* regexps though, presumably legitimately (I would 
be ridiculous to assume I'm the only one sane, although sometimes my 
confidence is weak :( ).

> The point here, and here:
> 
> > 3. Missing "$" prefixes are added to the names of FP registers (shows how 
> >    frequently some of the targets are tested, hmm...).
> 
> is precisely that mips*-ecoff has severely bitrotted from lack of
> active interest, so who knows how good the current output actually is?
> There's a danger in simply making the testsuite match what the current
> ECOFF assembler does.

 These "$" prefixes were actually missing for ELF targets, specifically 
mipstx39*-*-* (the only target setting ${gpr_ilocks}); being embedded 
rather than workstation class devices, I wouldn't expect Toshiba chips in 
any ECOFF systems.

> In a similar vein, the difference between endiannesses is something
> that could easily go wrong, but there are separate tests that should
> pick that up.  I'm much more comfortable with the endianness bits.

 We have this endianness leeway in many testcases.  I think it's better to 
have it than to hardcode endianness in the cases, making the the other 
endianness completely untested.  Again, it *might* be possible to improve 
the test framework to dynamically select between alternatives, defined in 
some way, based on the default target's endianness, but it looks all but 
trivial to me.

> Overall the patch is improvement, so it's OK to apply despite
> the concerns above.

 Applied, thanks.  I think my changes are not of the kind that would 
discourage further improvements.

  Maciej

Re: [PATCH 06/15] MIPS/GAS/test: Adjust LD for multi-target testing

[Maciej W. Rozycki]

On Mon, 18 Oct 2010, Maciej W. Rozycki wrote:

> > The patch seems to contain some parts that make the matching more strict
> > and some that make it less so.  The former look good, but I'm less sure
> > about the latter.  For example, with something like:
> > 
> > [0-9a-f]+ <[^>]*> lw   a0,(0|4096)\(at\)
> > [      ]*[0-9a-f]+: [A-Z0-9_]*LO[A-Z0-9_]*     \.data(\+0xfffff000)?
> > 
> > the difference seems relatively important: on some targets the addend
> > must be in-place, on some it mustn't be.  It might make more sense
> > to have separate dumps for the two cases.  However, that sort of
> > difference is probably picked up by other tests too, so the risk
> > might not be great.
> 
>  This is certainly strictier than the original that for the record was:
> 
> [0-9a-f]+ <[^>]*> lw	a0,0\(at\)
> [ 	]*[0-9a-f]+: [A-Z0-9_]*LO[A-Z0-9_]*	.data.*
> 
> (and obviously failed for ECOFF because of the different in-place addend).

 Wrong quotation actually, taken from the bit-rotten ld-ilocks.d -- the 
typical patterns this addresses were ones like these in ld.d:

[0-9a-f]+ <[^>]*> lw	a0,[-0-9]+\(at\)
[ 	]*[0-9a-f]+: [A-Z0-9_]*LO[A-Z0-9_]*	.data.*

that have been clearly adjusted for ECOFF and matched any in-place as well 
as relocation addends.  The new patterns are no less strict and then the 
rest of my consideration applies.

  Maciej