Re: PATCH: Set xfail on empic for Linux/mips

[cgd at broadcom dot com]

At Mon, 30 Sep 2002 14:17:16 -0700, H. J. Lu wrote:
> Since Linux doesn't use embedded pic, I set them xfail on Linux/mips.
> I will set they pass when they are fixed.

uh, if you're going to do that, why not just avoid testing them on
Linux/mips to begin with.

By setting them XFAIL, what'll happen is that they're broken one way
now, and future changes will cause them to be broken in additional
ways in the future, and somebody who's looking at it N months from now
won't be able to tell one from the other.

What you're proposing here amounts to saying "They don't work for me
right now, so the test is wrong and will be marked expected to fail."
Except, that's not the reality.  These tests are recently broken.
They used to work.  They should work.  If the new assembler linker is
correct, the tests should be adjusted.  If not, there is a real bug.
They _should not_ however, be marked XFAIL and be left alone.

If your policy is "no failures in your source tree," well, then, this
seems like a perfect thing for a local modification...


> FYI, Chris, ld-mips-elf/mips-elf.exp passed for ELF/mips for me now.
> I took a look at the tests. They are ELF/mips specific. There is no
> way they can pass for Linux/mips, ASIS.

Uh, only for values of the words "no way" that indicate "a way".  8-)

I hacked the ld-mips-elf/mips-elf.exp list like:

Index: ld/testsuite/ld-mips-elf//mips-elf.exp
===================================================================
RCS file: /cvs/src/src/ld/testsuite/ld-mips-elf/mips-elf.exp,v
retrieving revision 1.5
diff -u -p -r1.5 mips-elf.exp
--- ld/testsuite/ld-mips-elf//mips-elf.exp	18 Sep 2002 20:50:47 -0000	1.5
+++ ld/testsuite/ld-mips-elf//mips-elf.exp	30 Sep 2002 23:15:59 -0000
@@ -38,7 +38,9 @@ if { [istarget mips*-*-*] } then {
 	# Check generation of embedded relocs section.
 	run_dump_test "emrelocs-eb"
 	run_dump_test "emrelocs-el"
+    }
 
+    if { [istarget mips*-*-elf] || [istarget mips*-*-linux*] } {
 	run_dump_test "mips16-1"
 
 	run_dump_test "region1"

just to see how things would go.

Granted, region1 and the branch-misc-N tests did not in fact pass.

However, mips16-1 actually _did_ pass as-is.


Also (recognizing that this has no bearing on your claim that none
would pass "as-is") the branch tests did pass with minimal
modifications, specifically:

	* %s,4000,5000,g
	* add -N to link

because apparently:

	* the address they were linking at before overlapped w/
          .reginfo, and

	* w/o the -N, the link would fail like:

failed with: <./ld-new: tmpdir/dump: Not enough room for program headers (allocated 3, need 4)
./ld-new: final link failed: Bad value>, expected: <>

	(which really seems like a bug to me, but "whatever.")



cgd
--
Chris Demetriou                                            Broadcom Corporation
Principal Design Engineer                     Broadband Processor Business Unit
  Any opinions expressed in this message are mine, not necessarily Broadcom's.