* [PATCH]: Set little endian flag when linking SH arch tests for little endian multilibs
@ 2006-07-21 9:55 Nick Clifton
2006-07-21 13:25 ` Andrew STUBBS
0 siblings, 1 reply; 6+ messages in thread
From: Nick Clifton @ 2006-07-21 9:55 UTC (permalink / raw)
To: binutils
Hi Guys,
I am applying the small patch below to make the SH specific linker
tests set the appropriate endian flag when linking the arch tests.
Without this, testing little endian SH multilibs was producing lots
of unexpected failures and untested tests because the linker and
assembler disagreed about endianness.
Cheers
Nick
ld/testsuite/ChangeLog
2006-07-21 Nick Clifton <nickc@redhat.com>
* ld-sh/arch/arch.exp (test_arch): Set the endian flag to suit the
multilib being tested.
Index: ld/testsuite/ld-sh/arch/arch.exp
===================================================================
RCS file: /cvs/src/src/ld/testsuite/ld-sh/arch/arch.exp,v
retrieving revision 1.4
diff -c -3 -p -r1.4 arch.exp
*** ld/testsuite/ld-sh/arch/arch.exp 12 May 2005 07:32:08 -0000 1.4
--- ld/testsuite/ld-sh/arch/arch.exp 21 Jul 2006 09:53:25 -0000
*************** proc test_arch { file1 file2 arch result
*** 76,84 ****
set name2 [file tail $file2]
set rootname2 [file rootname $name2]
# This must use -r to prevent LD trying to relocate the (unrealistic) file
! send_log "$LD -r -o ${rootname1}_${rootname2}.o $file1 $file2\n"
! catch "exec $LD -r -o ${rootname1}_${rootname2}.o $file1 $file2" ld_output
send_log $ld_output
if {[string equal $ld_output ""] == 1} then {
--- 76,86 ----
set name2 [file tail $file2]
set rootname2 [file rootname $name2]
+ set flags [big_or_little_endian]
+
# This must use -r to prevent LD trying to relocate the (unrealistic) file
! send_log "$LD $flags -r -o ${rootname1}_${rootname2}.o $file1 $file2\n"
! catch "exec $LD $flags -r -o ${rootname1}_${rootname2}.o $file1 $file2" ld_output
send_log $ld_output
if {[string equal $ld_output ""] == 1} then {
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH]: Set little endian flag when linking SH arch tests for little endian multilibs
2006-07-21 9:55 [PATCH]: Set little endian flag when linking SH arch tests for little endian multilibs Nick Clifton
@ 2006-07-21 13:25 ` Andrew STUBBS
2006-07-23 14:25 ` Nick Clifton
0 siblings, 1 reply; 6+ messages in thread
From: Andrew STUBBS @ 2006-07-21 13:25 UTC (permalink / raw)
To: Nick Clifton; +Cc: binutils
[-- Attachment #1: Type: text/plain, Size: 840 bytes --]
Nick Clifton wrote:
> I am applying the small patch below to make the SH specific linker
> tests set the appropriate endian flag when linking the arch tests.
> Without this, testing little endian SH multilibs was producing lots
> of unexpected failures and untested tests because the linker and
> assembler disagreed about endianness.
As it happens I was just in the process of preparing that exact same
patch (right down to the character - spooky).
I also had an issue with default_ld_assemble in lib/ld-lib.exp. It tries
to apply linker endian flags to the assembler. I have attached the patch
I was using. Did you not see this issue?
There is also an issue because not all SH architectures have a little
endian variant, and therefore not all instructions and relocations are
implemented in both endians.
Andrew Stubbs
[-- Attachment #2: testsuite-flags.patch --]
[-- Type: text/plain, Size: 2183 bytes --]
Index: src/ld/testsuite/lib/ld-lib.exp
===================================================================
--- src.orig/ld/testsuite/lib/ld-lib.exp 2006-07-05 12:28:54.000000000 +0100
+++ src/ld/testsuite/lib/ld-lib.exp 2006-07-05 14:24:10.000000000 +0100
@@ -276,6 +276,10 @@ proc default_ld_assemble { as source obj
if ![info exists ASFLAGS] { set ASFLAGS "" }
set flags [big_or_little_endian]
+ if {[istarget sh*-*-*] || [istarget pj*-*-*]} {
+ regsub -- {-EB} $flags {-big} flags
+ regsub -- {-EL} $flags {-little} flags
+ }
verbose -log "$as $flags $ASFLAGS -o $object $source"
Index: src/ld/testsuite/ld-sh/arch/arch.exp
===================================================================
--- src.orig/ld/testsuite/ld-sh/arch/arch.exp 2005-05-12 08:32:08.000000000 +0100
+++ src/ld/testsuite/ld-sh/arch/arch.exp 2006-07-05 16:25:05.000000000 +0100
@@ -76,9 +76,11 @@ proc test_arch { file1 file2 arch result
set name2 [file tail $file2]
set rootname2 [file rootname $name2]
+ set flags [big_or_little_endian]
+
# This must use -r to prevent LD trying to relocate the (unrealistic) file
- send_log "$LD -r -o ${rootname1}_${rootname2}.o $file1 $file2\n"
- catch "exec $LD -r -o ${rootname1}_${rootname2}.o $file1 $file2" ld_output
+ send_log "$LD $flags -r -o ${rootname1}_${rootname2}.o $file1 $file2\n"
+ catch "exec $LD $flags -r -o ${rootname1}_${rootname2}.o $file1 $file2" ld_output
send_log $ld_output
if {[string equal $ld_output ""] == 1} then {
@@ -117,9 +119,11 @@ proc test_arch_error { file1 file2 resul
set name2 [file tail $file2]
set rootname2 [file rootname $name2]
+ set flags [big_or_little_endian]
+
# This must use -r to prevent LD trying to relocate the (unrealistic) file
- send_log "$LD -r -o ${rootname1}_${rootname2}.o $file1 $file2\n"
- catch "exec $LD -r -o ${rootname1}_${rootname2}.o $file1 $file2" ld_output
+ send_log "$LD $flags -r -o ${rootname1}_${rootname2}.o $file1 $file2\n"
+ catch "exec $LD $flags -r -o ${rootname1}_${rootname2}.o $file1 $file2" ld_output
send_log $ld_output
if {[string equal $ld_output ""] == 1} then {
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH]: Set little endian flag when linking SH arch tests for little endian multilibs
2006-07-21 13:25 ` Andrew STUBBS
@ 2006-07-23 14:25 ` Nick Clifton
2006-07-24 8:32 ` Andrew STUBBS
0 siblings, 1 reply; 6+ messages in thread
From: Nick Clifton @ 2006-07-23 14:25 UTC (permalink / raw)
To: Andrew STUBBS; +Cc: binutils
Hi Andrew,
> As it happens I was just in the process of preparing that exact same
> patch (right down to the character - spooky).
[cue X-Files music...]
> I also had an issue with default_ld_assemble in lib/ld-lib.exp. It tries
> to apply linker endian flags to the assembler. I have attached the patch
> I was using. Did you not see this issue?
Yes I did. My patches were only the start, but I ran out of time to
track down all the other causes of little-endian SH linker test
failures. Fortunately it appears that you did not...
> There is also an issue because not all SH architectures have a little
> endian variant, and therefore not all instructions and relocations are
> implemented in both endians.
Yes, I suspected this, but again I did not have time to look into it.
> +++ src/ld/testsuite/lib/ld-lib.exp 2006-07-05 14:24:10.000000000 +0100
> @@ -276,6 +276,10 @@ proc default_ld_assemble { as source obj
> if ![info exists ASFLAGS] { set ASFLAGS "" }
>
> set flags [big_or_little_endian]
> + if {[istarget sh*-*-*] || [istarget pj*-*-*]} {
> + regsub -- {-EB} $flags {-big} flags
> + regsub -- {-EL} $flags {-little} flags
> + }
I do not really like this. I think that it would be better to avoid
having target specific knowledge in ld-lib.exp, where possible anyway.
How about having a new board_info variable which, if defined, supplies
the name of the little- or big- endian switch to use ? eg (untested):
proc big_or_little_endian {} {
if [board_info [target_info name] exists multilib_flags] {
set tmp_flags " [board_info [target_info name] multilib_flags]"
foreach x $tmp_flags {
case $x in {
{*big*endian eb EB -eb -EB -mb} {
+ if [board_info [target_info name] exists
bigendian_switch] {
+ set flags [board_info [target_info name]
bigendian_switch]
+ } else {
set flags " -EB"
+ }
return $flags
}
{*little*endian el EL -el -EL -ml} {
+ if [board_info [target_info name] exists
littleendian_switch] {
+ set flags [board_info [target_info name]
littleendian_switch]
+ } else {
set flags " -EL"
+ }
return $flags
}
}
}
}
set flags ""
return $flags
}
Then we can just define bigendian_switch and littleendian_switch in
sh-hms.exp (or whatever dejagnu board file is being used).
> @@ -117,9 +119,11 @@ proc test_arch_error { file1 file2 resul
> set name2 [file tail $file2]
> set rootname2 [file rootname $name2]
>
> + set flags [big_or_little_endian]
> +
> # This must use -r to prevent LD trying to relocate the (unrealistic) file
> - send_log "$LD -r -o ${rootname1}_${rootname2}.o $file1 $file2\n"
> - catch "exec $LD -r -o ${rootname1}_${rootname2}.o $file1 $file2" ld_output
> + send_log "$LD $flags -r -o ${rootname1}_${rootname2}.o $file1 $file2\n"
> + catch "exec $LD $flags -r -o ${rootname1}_${rootname2}.o $file1 $file2" ld_output
> send_log $ld_output
>
> if {[string equal $ld_output ""] == 1} then {
For reasons I do not understand I found that if I included the above
part of the patch to arch.exp I ended up with *more* SH linker testsuite
failures (for little endian multilibs) than without it. This was
another thing that I was going to investigate when I had more time. If
you can show that it does reduce the number of testsuite failures
however then I would be happy to accept it.
Cheers
Nick
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH]: Set little endian flag when linking SH arch tests for little endian multilibs
2006-07-23 14:25 ` Nick Clifton
@ 2006-07-24 8:32 ` Andrew STUBBS
2006-07-24 10:42 ` Andrew STUBBS
0 siblings, 1 reply; 6+ messages in thread
From: Andrew STUBBS @ 2006-07-24 8:32 UTC (permalink / raw)
To: Nick Clifton; +Cc: binutils
Nick Clifton wrote:
> How about having a new board_info variable which, if defined, supplies
> the name of the little- or big- endian switch to use ? eg (untested):
[snip]
> Then we can just define bigendian_switch and littleendian_switch in
> sh-hms.exp (or whatever dejagnu board file is being used).
The problem is that the sh linker continues to take -EB/-EL, but the sh
assembler uses -big/-little. According to my examination of the code the
pj assembler is the same.
Your solution looks good, but requires separate variables for assembler
and linker, and then the the big_or_little_endian function needs to know
which is needed.
BTW, Dejagnu 1.4.4 uses sh-sim, not sh-hms.
>> @@ -117,9 +119,11 @@ proc test_arch_error { file1 file2 resul
>> set name2 [file tail $file2]
>> set rootname2 [file rootname $name2]
>>
>> + set flags [big_or_little_endian]
>> +
>> # This must use -r to prevent LD trying to relocate the
>> (unrealistic) file
>> - send_log "$LD -r -o ${rootname1}_${rootname2}.o $file1 $file2\n"
>> - catch "exec $LD -r -o ${rootname1}_${rootname2}.o $file1 $file2"
>> ld_output
>> + send_log "$LD $flags -r -o ${rootname1}_${rootname2}.o $file1
>> $file2\n"
>> + catch "exec $LD $flags -r -o ${rootname1}_${rootname2}.o $file1
>> $file2" ld_output
>> send_log $ld_output
>>
>> if {[string equal $ld_output ""] == 1} then {
>
> For reasons I do not understand I found that if I included the above
> part of the patch to arch.exp I ended up with *more* SH linker testsuite
> failures (for little endian multilibs) than without it. This was
> another thing that I was going to investigate when I had more time. If
> you can show that it does reduce the number of testsuite failures
> however then I would be happy to accept it.
Hmmm, did you use both parts of the patch? You say that I have had time
to test this, but actually the reason I had not submitted the patch was
that I had not yet finished testing it. I suspect that a few more bits
are needed. I also have a few more adjustments to the arch.exp script to
submit. Also, this has got put on the back burner for the last few weeks
so it isn't fresh in my mind any more.
I'll look into it. Not sure when though.
Andrew
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH]: Set little endian flag when linking SH arch tests for little endian multilibs
2006-07-24 8:32 ` Andrew STUBBS
@ 2006-07-24 10:42 ` Andrew STUBBS
2006-07-24 13:55 ` Nick Clifton
0 siblings, 1 reply; 6+ messages in thread
From: Andrew STUBBS @ 2006-07-24 10:42 UTC (permalink / raw)
To: Nick Clifton; +Cc: binutils
Andrew Stubbs wrote:
>> For reasons I do not understand I found that if I included the above
>> part of the patch to arch.exp I ended up with *more* SH linker
>> testsuite failures (for little endian multilibs) than without it.
>> This was another thing that I was going to investigate when I had more
>> time. If you can show that it does reduce the number of testsuite
>> failures however then I would be happy to accept it.
>
> Hmmm, did you use both parts of the patch? You say that I have had time
> to test this, but actually the reason I had not submitted the patch was
> that I had not yet finished testing it. I suspect that a few more bits
> are needed. I also have a few more adjustments to the arch.exp script to
> submit. Also, this has got put on the back burner for the last few weeks
> so it isn't fresh in my mind any more.
>
> I'll look into it. Not sure when though.
Well, I did the tests with and without my two-part patch.
Without:
=== ld Summary for sh-sim ===
# of expected passes 853
# of unexpected failures 15
# of expected failures 1
# of unresolved testcases 1
# of untested testcases 21
=== ld Summary for sh-sim/-ml ===
# of expected passes 133
# of unexpected failures 357
# of expected failures 1
# of unresolved testcases 14
# of untested testcases 378
=== ld Summary ===
# of expected passes 986
# of unexpected failures 372
# of expected failures 2
# of unresolved testcases 15
# of untested testcases 399
With:
=== ld Summary for sh-sim ===
# of expected passes 853
# of unexpected failures 15
# of expected failures 1
# of unresolved testcases 1
# of untested testcases 21
=== ld Summary for sh-sim/-ml ===
# of expected passes 797
# of unexpected failures 80
# of expected failures 1
# of untested testcases 21
=== ld Summary ===
# of expected passes 1650
# of unexpected failures 95
# of expected failures 2
# of unresolved testcases 1
# of untested testcases 42
So that's quite considerably better. I have not looked at what causes
the 65 additional failures in little endian, and I do not know when I
will get chance to. I do know that the assembler tests also fail some
little endian due to a lack of support for it in certain target
variants. At least I now know why I never got around to submitting the
patch.
The other testsuite patches I alluded to are, it turns out, related to
changes in behaviour caused by another patch I have not yet submitted,
so you don't need to worry about that just yet.
Andrew Stubbs
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH]: Set little endian flag when linking SH arch tests for little endian multilibs
2006-07-24 10:42 ` Andrew STUBBS
@ 2006-07-24 13:55 ` Nick Clifton
0 siblings, 0 replies; 6+ messages in thread
From: Nick Clifton @ 2006-07-24 13:55 UTC (permalink / raw)
To: Andrew STUBBS; +Cc: binutils
Hi Andrew,
>>> For reasons I do not understand I found that if I included the above
>>> part of the patch to arch.exp I ended up with *more* SH linker
>>> testsuite failures (for little endian multilibs) than without it.
>> Hmmm, did you use both parts of the patch?
Well I used my version of it, but essentially yes.
> Well, I did the tests with and without my two-part patch.
>
> Without:
> === ld Summary for sh-sim/-ml ===
>
> # of expected passes 133
> # of unexpected failures 357
> # of expected failures 1
> # of unresolved testcases 14
> # of untested testcases 378
> With:
> === ld Summary for sh-sim/-ml ===
>
> # of expected passes 797
> # of unexpected failures 80
> # of expected failures 1
> # of untested testcases 21
> So that's quite considerably better. I have not looked at what causes
> the 65 additional failures in little endian,
This is where I thought that the second part of the patch was giving
problems. Without it I get only 33 unexpected failures for the "-ml"
multilib... I suspect however that this was connected to the fact that
some architecture variants do not support little endian and the tests do
not current make allowances for this.
Anyway thanks very much for looking into this, and I am sure that at
some date somebody will finish off the work and fix the remaining failures.
Cheers
Nick
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2006-07-24 13:55 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2006-07-21 9:55 [PATCH]: Set little endian flag when linking SH arch tests for little endian multilibs Nick Clifton
2006-07-21 13:25 ` Andrew STUBBS
2006-07-23 14:25 ` Nick Clifton
2006-07-24 8:32 ` Andrew STUBBS
2006-07-24 10:42 ` Andrew STUBBS
2006-07-24 13:55 ` Nick Clifton
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).