Re: [PATCH]: Set little endian flag when linking SH arch tests for little endian multilibs

[Nick Clifton <nickc at redhat dot com>]

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