Re: [PATCH 04/11] Little-endian fixes: VSX tests and pseudo-regs

[Joel Brobecker <brobecker at adacore dot com>]

> ChangeLog:
> 
> 	* rs6000-tdep.c (efpr_pseudo_register_read): Use correct offset
> 	of the overlapped FP register within the VSX register on little-
> 	endian platforms.
> 	(efpr_pseudo_register_write): Likewise.
> 
> testsuite/ChangeLog:
> 
> 	* gdb.arch/vsx-regs.exp: Check target endianness.  Provide variants
> 	of the test patterns for use on little-endian systems.

I only have small comments on this patch...

> 
> 
> Index: binutils-gdb/gdb/rs6000-tdep.c
> ===================================================================
> --- binutils-gdb.orig/gdb/rs6000-tdep.c
> +++ binutils-gdb/gdb/rs6000-tdep.c
> @@ -2781,7 +2781,8 @@ efpr_pseudo_register_read (struct gdbarc
>    int reg_index = reg_nr - tdep->ppc_efpr0_regnum;
>  
>    /* Read the portion that overlaps the VMX register.  */
> -  return regcache_raw_read_part (regcache, tdep->ppc_vr0_regnum + reg_index, 0,
> +  int offset = gdbarch_byte_order (gdbarch) == BFD_ENDIAN_BIG ? 0 : 8;
> +  return regcache_raw_read_part (regcache, tdep->ppc_vr0_regnum + reg_index, offset,
>  				 register_size (gdbarch, reg_nr), buffer);

Can you add an empty line after the local variable declaration?
Also, the return statement now has a line that's too long, and so
needs to be broken up.

> @@ -2794,7 +2795,8 @@ efpr_pseudo_register_write (struct gdbar
>    int reg_index = reg_nr - tdep->ppc_efpr0_regnum;
>  
>    /* Write the portion that overlaps the VMX register.  */
> -  regcache_raw_write_part (regcache, tdep->ppc_vr0_regnum + reg_index, 0,
> +  int offset = gdbarch_byte_order (gdbarch) == BFD_ENDIAN_BIG ? 0 : 8;
> +  regcache_raw_write_part (regcache, tdep->ppc_vr0_regnum + reg_index, offset,
>  			   register_size (gdbarch, reg_nr), buffer);

Same here.

> Index: binutils-gdb/gdb/testsuite/gdb.arch/vsx-regs.exp
> ===================================================================
> --- binutils-gdb.orig/gdb/testsuite/gdb.arch/vsx-regs.exp
> +++ binutils-gdb/gdb/testsuite/gdb.arch/vsx-regs.exp
> @@ -58,19 +58,46 @@ if ![runto_main] then {
>      gdb_suppress_tests
>  }
>  
> +send_gdb "show endian\n"
> +set endianness ""
> +gdb_expect {
> +    -re "(The target endianness is set automatically .currently )(big|little)( endian.*)$gdb_prompt $" {
> +        pass "endianness"
> +        set endianness $expect_out(2,string)
> +    }
> +    -re ".*$gdb_prompt $" {
> +        fail "couldn't get endianness"
> +    }
> +    timeout             { fail "(timeout) endianness" }

Can you use test_gdb_multiple instead? That should allow you to simplify
this a little by not having to handle the timeout. Generally speaking,
we now prefer not to use send_gdb+gdb_expect unless we absolutely have to.

-- 
Joel