On 07/21/2011 01:55 AM, Pedro Alves wrote:
> On Wednesday 20 July 2011 03:09:18, Yao Qi wrote:
> 
>> +static CORE_ADDR
>> +tic6x_get_pc (struct regcache *regcache)
>> +{
>> +  union tic6x_register pc;
>> +  collect_register_by_name (regcache, "PC", pc.buf);
> 
> Empty line missing after declaration.  At least one more instance.
> 

Fixed it here and other places.

>> +int *tic6x_regmap;
>> +
> ...
>> +unsigned int tic6x_breakpoint;
> 
> Could be static?
> 

Yes, they can be static.  Fixed.

>>  s390-linux32-expedite = r14,r15,pswa
>>  s390-linux64-expedite = r14l,r15l,pswa
>>  s390x-linux64-expedite = r14,r15,pswa
>> +tic6x-linux-expedite = A15,PC
> 
> Stupid question: is there a reason all register
> names are all caps in C6x?
> 

Because all register names are capitalized in TI's reference guide and
"TMS320C6000 Assembly Language Tools User's Guide".

>> +++ b/gdb/features/tic6x-linux.xml
>> @@ -0,0 +1,12 @@
>> +<?xml version="1.0"?>
>> +<!-- Copyright (C) 2010, 2011 Free Software Foundation, Inc.
>> +
>> +     Copying and distribution of this file, with or without modification,
>> +     are permitted in any medium without royalty provided the copyright
>> +     notice and this notice are preserved.  -->
>> +
>> +<!DOCTYPE target SYSTEM "gdb-target.dtd">
>> +<target>
>> +  <architecture>tic6x</architecture>
> 
> Since this is for linux only, add:
> 
>  <osabi>GNU/Linux</osabi>
> 
>> +  <xi:include href="tic6x-cpu.xml"/>
>> +</target>
> 

Done.

>> +#elif defined(__TMS320C6X__)
>> +#define PT_TEXT_ADDR     0x10000*4
>> +#define PT_DATA_ADDR     0x10004*4
>> +#define PT_TEXT_END_ADDR 0x10008*4
> 
> Surround with parens as principle to avoid precedence
> surprises:
> 
>   #define PT_TEXT_ADDR     (0x10000*4)
> 

Done.

>> +<!DOCTYPE feature SYSTEM "gdb-target.dtd">
>> +<feature name="org.gnu.gdb.tic6x.cpu">
>> +  <reg name="A0" bitsize="32"/>
>> +  <reg name="A1" bitsize="32"/>
>> +  <reg name="A2" bitsize="32"/>
> 
> ...
> 
> These are all general purpose, core registers, right?
> 

Not really.  A0-31 and B0-31 are general purpose registers, even there
are some difference among different C6x cores (such as C62x, C67x, and
C64x).  The rest of them can be regarded as control registers and status
registers.  The target description should be refined for the different
C6x cores, and I'd like to do it in follow-up patches later.  Is it OK?

-- 
Yao (齐尧)