Bug 20936 - provide sparc and sparcv9 target description XML files

  On 12.12.2016 13:53, Yao Qi wrote:

> Hi Ivo,
> Your patch does two orthogonal things IMO,
>
>   - Pseudo register support enhancement, patch #1
>   - XML target description support and sparc*-tdep.c updates, patch #2,
>
> Can you split them to two patches?

Hi Yao,

Thank you for looking into my changes.
I think I can see your motivation behind splitting the patch into two,
for better readability and manageability.
However my intention was only to provide support for registers
supplied by target description (Valgrind shadow registers in addition
to sparc real hardware ones).
Pseudo register changes were just a necessity to get this done
because Valgrind shadow registers are considered as "real" registers
and thus they are numbered before any pseudo registers.
This also means I cannot draw a clear line between your suggested
patch #1 and #2. Can you take my whole patch as is?

> I prefer to using register names provided by target description, does
> the code below work for you?
>
>    if (tdesc_has_registers (gdbarch_target_desc (gdbarch)))
>      return tdesc_register_name (gdbarch, rawnum);
>    else if (regnum >= 0 && regnum < gdbarch_num_regs (gdbarch))
>      return sparc32_register_names[regnum];
>    else
>      return sparc32_pseudo_register_name (gdbarch, regnum);

Yes, it does. Fixed also in other functions returning register names and types.

>> -  /* Pseudo registers.  */
>> +/* Pseudo registers.  */
>> +enum sparc32_pseudo_regnum
>> +{
>>     SPARC32_D0_REGNUM,		/* %d0 */
> Explicitly set it to zero.

Fixed.

>> @@ -969,19 +1002,24 @@ sparc64_store_arguments (struct regcache
>>   
>>   	  /* If we're storing the value in a floating-point register,
>>                also store it in the corresponding %0 register(s).  */
>> -	  if (regnum >= SPARC64_D0_REGNUM && regnum <= SPARC64_D10_REGNUM)
>> -	    {
>> -	      gdb_assert (element < 6);
>> -	      regnum = SPARC_O0_REGNUM + element;
>> -	      regcache_cooked_write (regcache, regnum, valbuf);
>> -	    }
>> -	  else if (regnum >= SPARC64_Q0_REGNUM && regnum <= SPARC64_Q8_REGNUM)
>> -	    {
>> -	      gdb_assert (element < 5);
>> -	      regnum = SPARC_O0_REGNUM + element;
>> -	      regcache_cooked_write (regcache, regnum, valbuf);
>> -	      regcache_cooked_write (regcache, regnum + 1, valbuf + 8);
>> -	    }
>> +	  if (regnum >= gdbarch_num_regs (gdbarch))
>> +            {
> The indentation looks odd to me.

Indeed, just looking at the patch it does. This is because the code 
mixes spaces and tabs and I wanted to preserve as much of existing code 
as possible, to keep the patch size down. Please look at the resulting 
file - indentation is ok.

>
>>         /* Always store the argument in memory.  */
>> diff -Npur a/gdb/testsuite/gdb.xml/tdesc-regs.exp b/gdb/testsuite/gdb.xml/tdesc-regs.exp
>> --- a/gdb/testsuite/gdb.xml/tdesc-regs.exp	2016-02-09 19:19:39.000000000 +0000
>> +++ b/gdb/testsuite/gdb.xml/tdesc-regs.exp	2016-12-06 15:53:35.418621207 +0000
>> @@ -49,6 +49,12 @@ switch -glob -- [istarget] {
>>       "s390*-*-*" {
>>   	set core-regs {s390-core32.xml s390-acr.xml s390-fpr.xml}
>>       }
>> +    "sparc-*-*" {
>> +        set core-regs {sparc32-cpu.xml sparc32-fpu.xml sparc32-cp0.xml}
>> +    }
>> +    "sparc64-*-*" {
>> +        set core-regs {sparc64-cpu.xml sparc64-fpu.xml sparc64-cp0.xml}
>> +    }
> You need 'set regdir "sparc/"'

Fixed.

>
>> 	* sparc32-cp0.xml, sparc32-cpu.xml, sparc32-fpu.xml: New files for
>> 	sparc 32-bit.
>> 	* sparc64-cp0.xml, sparc64-cpu.xml, sparc64-fpu.xml: New files
>> 	for sparc 64-bit.
>> 	* sparc32-solaris.xml, sparc64-solaris.xml: New files for sparc32
>> 	and sparc64 on Solaris.
>> 	* sparc-solaris.c, sparc64-solaris.c: Generated.
> These file names should be prefixed with "feature/sparc/"

Fixed.

>> 	* sparc-tdep.h: Deal with sparc32 and sparc64 differences
>> 	in target descriptions. Separate real and pseudo registers.
>> 	* sparc-tdep.c: Validate and use registers of the target description.
>> 	Pseudo registers are numbered after all real registers from the target
>> 	description; deal with it.
>> 	* sparc64-tdep.h: Separate real and pseudo registers.
>> 	* sparc64-tdep.c: Pseudo registers are numbered after all real
>> 	registers from the target description; deal with it.
> Please describe the change on the function level, take a look at
> https://sourceware.org/gdb/wiki/ContributionChecklist there are two
> sections about ChangeLog.

Fixed.

>
>> 	* gdb.texinfo: New node "Sparc Features".
> This should go to a separated entry,
>
> gdb/doc:
>
> 2016-12-07  Ivo Raisr  <ivo.raisr@oracle.com>
>
> 	* gdb.texinfo (Standard Target Features): Document SPARC features.
> 	(Sparc Features): New node.

Fixed.

>
>> 	* tdesc-regs.exp: Provide sparc core registers for the tests.
> Likewise,
>
> gdb/testsuite:
>
> 2016-12-07  Ivo Raisr  <ivo.raisr@oracle.com>
>
> 	* gdb.xml/tdesc-regs.exp: Provide sparc core registers for
> 	the tests.

Fixed.

Please find a new patch and 3 new ChangeLog entries. Let me know your 
comments.

I.

ChangeLog entry:
2017-01-04  Ivo Raisr  <ivo.raisr@oracle.com>

	PR tdep/20936
	* gdb.texinfo: (Standard Target Features): Document SPARC features.
	(Sparc Features): New node.
ChangeLog entry:
2017-01-04  Ivo Raisr  <ivo.raisr@oracle.com>

	PR tdep/20936
	Provide and use sparc32 and sparc64 target description XML files.
	* feature/sparc/sparc32-cp0.xml, feature/sparc/sparc32-cpu.xml,
	feature/sparc/sparc32-fpu.xml: New files for sparc 32-bit.
	* feature/sparc/sparc64-cp0.xml, feature/sparc/sparc64-cpu.xml,
	feature/sparc/sparc64-fpu.xml: New files for sparc 64-bit.
	* sparc32-solaris.xml, sparc64-solaris.xml: New files for sparc32
	and sparc64 on Solaris.
	* sparc-solaris.c, sparc64-solaris.c: Generated.
	* sparc-tdep.h: Deal with sparc32 and sparc64 differences
	in target descriptions. Separate real and pseudo registers.
	* sparc64-tdep.h: Separate real and pseudo registers.
	* sparc-tdep.c (sparc32_pseudo_register_name): New function returning
	pseudo register names on sparc 32-bit.
	(sparc32_register_name): Account for register names provided by the
	target description.
	(sparc32_pseudo_register_type): New function returning pseudo register
	types on sparc 32-bit.
	(sparc32_register_type): Account for register types provided by the
	target description.
	(sparc32_pseudo_register_read, sparc32_pseudo_register_write): Pseudo
	registers may have different numbers based on the target description.
	(validate_tdesc_registers): Validate registers provided by the target
	description.
	(sparc32_gdbarch_init): Use new functions sparc32_pseudo_register_name
	and sparc32_pseudo_register_type. Validate and use registers provided
	by the target description.
	* sparc64-tdep.c (sparc64_pseudo_register_name): New function returning
	pseudo register names on sparc 64-bit.
	(sparc64_register_name): Account for register names provided by the
	target description.
	(sparc64_pseudo_register_type): New function returning pseudo register
	types on sparc 64-bit.
	(sparc64_register_type): Account for register types provided by the
	target description.
	(sparc64_pseudo_register_read, sparc64_pseudo_register_write): Pseudo
	registers may have different numbers based on the target description.
	(sparc64_store_floating_fields, sparc64_extract_floating_fields,
	sparc64_store_arguments): Pseudo registers may have different numbers
	based on the target description; deal with it.
	(sparc64_init_abi): Use new functions sparc64_pseudo_register_name
	and sparc64_pseudo_register_type.
ChangeLog entry:
2017-01-04  Ivo Raisr  <ivo.raisr@oracle.com>

	PR tdep/20936
	* gdb.xml/tdesc-regs.exp: Provide sparc core registers for
	the tests.

Bug 20936 - provide sparc and sparcv9 target description XML files

Commit Message

Patch