[PATCH 14/15] MIPS/GAS/test: Run the LD test with forward references

[PATCH 14/15] MIPS/GAS/test: Run the LD test with forward references

[Maciej W. Rozycki]

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

Hi,

 This change extends the LD test to run with forward references.  
Effectively this tests relaxation, covering the original problem fixed 
earlier in this series.

 Note this has revealed a deficiency in our relaxation code.  In theory 
the machine code generated from a single piece of assembly source should 
be the same regardless of whether any data objects referenced have been 
provided before the respective references or after.  This is not the case.  
A couple of unnecesary load-delay NOPs are output for platforms that have
load-delay slots.

 The reason for this is the load-delay slot dependency is only checked 
against the first relaxation variant and any requirements are fulfilled by 
appending these NOPs (or one only, as other cases such as HI/LO 
dependencies do not apply here) to the code before the variant frag, 
rather then the relaxed sequence itself.  If in the end the second variant 
is chosen then the stray NOP remains.

 Any particular reason we take this approach these days?  I understand 
with the old relaxation code resolving such things was somewhat tricky if 
possible at all, but I see no reason for the extra NOP to be emitted to 
the variant frags as a need arises these days.  In no case it would seem 
the dependency would cross beyond a variant frag to any code that follows 
so no need to fear choosing the second variant would cause any NOPs to be 
missing to satisfy code after the frag it would seem (to me anyway).

 Anyway, until (unless) this is resolved, I have prepared a separate dump. 
mips1@ld-f.d, to cover this specific case; emitted code is not incorrect, 
it's just inefficient.  Normally it should be the same as (or simply 
include) mips1@ld.d.

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

	gas/testsuite/
	* gas/mips/ld.s: Adjust to let data objects be only 
	defined/declared (as appropriate) at the end of assembly, based
	on the presence or not of the "forward" symbol.
	* gas/mips/ld-f.d: New test.
	* gas/mips/mips1@ld-f.d: Likewise. MIPS I version.
	* gas/mips/r3000@ld-f.d: Likewise, R3000 version.
	* gas/mips/ecoff@ld-f.d: Likewise, ECOFF version.
	* gas/mips/r3900@ecoff@ld-f.d: Likewise, R3900/ECOFF version.
	* gas/mips/mips2@ecoff@ld-f.d: Likewise, MIPS II/ECOFF version.
	* gas/mips/mips32@ecoff@ld-f.d: Likewise, MIPS32/ECOFF version.
	* gas/mips/mips32r2@ecoff@ld-f.d: Likewise, MIPS32r2/ECOFF 
	version.
	* gas/mips/ld-n32-f.d: New test.
	* gas/mips/ld-n64-f.d: Likewise.
	* gas/mips/mips.exp: Run the new tests.

 OK to apply?

  Maciej

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

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

Re: [PATCH 14/15] MIPS/GAS/test: Run the LD test with forward references

[Richard Sandiford]

"Maciej W. Rozycki" <macro@linux-mips.org> writes:
>  Note this has revealed a deficiency in our relaxation code.  In theory 
> the machine code generated from a single piece of assembly source should 
> be the same regardless of whether any data objects referenced have been 
> provided before the respective references or after.  This is not the case.  
> A couple of unnecesary load-delay NOPs are output for platforms that have
> load-delay slots.
>
>  The reason for this is the load-delay slot dependency is only checked 
> against the first relaxation variant and any requirements are fulfilled by 
> appending these NOPs (or one only, as other cases such as HI/LO 
> dependencies do not apply here) to the code before the variant frag, 
> rather then the relaxed sequence itself.  If in the end the second variant 
> is chosen then the stray NOP remains.
>
>  Any particular reason we take this approach these days?  I understand 
> with the old relaxation code resolving such things was somewhat tricky if 
> possible at all, but I see no reason for the extra NOP to be emitted to 
> the variant frags as a need arises these days.  In no case it would seem 
> the dependency would cross beyond a variant frag to any code that follows 
> so no need to fear choosing the second variant would cause any NOPs to be 
> missing to satisfy code after the frag it would seem (to me anyway).

No particular reason, no.  It just wasn't part of the motivation for
the original relaxation changes.  Improving the quality of MIPS I code
is always very low down the priority list.

> 2010-10-03  Maciej W. Rozycki  <macro@linux-mips.org>
>
> 	gas/testsuite/
> 	* gas/mips/ld.s: Adjust to let data objects be only 
> 	defined/declared (as appropriate) at the end of assembly, based
> 	on the presence or not of the "forward" symbol.
> 	* gas/mips/ld-f.d: New test.
> 	* gas/mips/mips1@ld-f.d: Likewise. MIPS I version.
> 	* gas/mips/r3000@ld-f.d: Likewise, R3000 version.
> 	* gas/mips/ecoff@ld-f.d: Likewise, ECOFF version.
> 	* gas/mips/r3900@ecoff@ld-f.d: Likewise, R3900/ECOFF version.
> 	* gas/mips/mips2@ecoff@ld-f.d: Likewise, MIPS II/ECOFF version.
> 	* gas/mips/mips32@ecoff@ld-f.d: Likewise, MIPS32/ECOFF version.
> 	* gas/mips/mips32r2@ecoff@ld-f.d: Likewise, MIPS32r2/ECOFF 
> 	version.
> 	* gas/mips/ld-n32-f.d: New test.
> 	* gas/mips/ld-n64-f.d: Likewise.
> 	* gas/mips/mips.exp: Run the new tests.

Let's use "-forward" rather than "-f", for consistency with the
symbol name and to avoid any possible confusion with "floating point".

OK otherwise, thanks.

Richard

Re: [PATCH 14/15] MIPS/GAS/test: Run the LD test with forward references

[Maciej W. Rozycki]

On Sun, 10 Oct 2010, Richard Sandiford wrote:

> >  Any particular reason we take this approach these days?  I understand 
> > with the old relaxation code resolving such things was somewhat tricky if 
> > possible at all, but I see no reason for the extra NOP to be emitted to 
> > the variant frags as a need arises these days.  In no case it would seem 
> > the dependency would cross beyond a variant frag to any code that follows 
> > so no need to fear choosing the second variant would cause any NOPs to be 
> > missing to satisfy code after the frag it would seem (to me anyway).
> 
> No particular reason, no.  It just wasn't part of the motivation for
> the original relaxation changes.  Improving the quality of MIPS I code
> is always very low down the priority list.

 It depends for whom. ;)  I do accept the reality though.  Thanks for the 
explanation.  It looks a little bit involving to me, so I'll defer it for 
the time being and see if I can squeeze it in the 2.22's timeframe. :)

> > 	gas/testsuite/
> > 	* gas/mips/ld.s: Adjust to let data objects be only 
> > 	defined/declared (as appropriate) at the end of assembly, based
> > 	on the presence or not of the "forward" symbol.
> > 	* gas/mips/ld-f.d: New test.
> > 	* gas/mips/mips1@ld-f.d: Likewise. MIPS I version.
> > 	* gas/mips/r3000@ld-f.d: Likewise, R3000 version.
> > 	* gas/mips/ecoff@ld-f.d: Likewise, ECOFF version.
> > 	* gas/mips/r3900@ecoff@ld-f.d: Likewise, R3900/ECOFF version.
> > 	* gas/mips/mips2@ecoff@ld-f.d: Likewise, MIPS II/ECOFF version.
> > 	* gas/mips/mips32@ecoff@ld-f.d: Likewise, MIPS32/ECOFF version.
> > 	* gas/mips/mips32r2@ecoff@ld-f.d: Likewise, MIPS32r2/ECOFF 
> > 	version.
> > 	* gas/mips/ld-n32-f.d: New test.
> > 	* gas/mips/ld-n64-f.d: Likewise.
> > 	* gas/mips/mips.exp: Run the new tests.
> 
> Let's use "-forward" rather than "-f", for consistency with the
> symbol name and to avoid any possible confusion with "floating point".
> 
> OK otherwise, thanks.

 Committed with your suggestion applied now, thanks.

  Maciej