Re: 68hc11/12/s12x/xgate patch

[James Murray <jsm at jsm-net dot demon dot co dot uk>]

On Tue, 2012-01-10 at 14:41 +0000, nick clifton wrote:
Hi James,
> 
> > Find attached my latest revision of a patch to the m68hc11 port
> 
> Thank you for submitting this.  Here are some observations and
requests 
> for improvements on the patch:
> 
> 
*) Are you offering to act as a maintainer for the S12X target ?
> 

I presume this means performing regular builds of the m68hc11 target as
CVS code progresses and ensuring it builds cleanly before each release?

I would be prepared to do that to the best of my ability.

I have some binutils knowledge, as demonstrated by the ability to add
this new code, but I am far from "expert" level.


*) You have included a patch to the top level configure file, but not 
> the top level configure.ac file (from which the configure file is 
> generated).
> 
> *) You have included a patch to the top level config.sub file.  Are
you 
> aware that this patch needs to be submitted to a different project ? 
> (config-patches@gnu.org)

I'll double check both of these to see if I really need to change them.
I posted a related question a week or so back - the m68hc11 port allows
itself to be built under a number of names, but I' unsure why it needs
to. The name does change the default runtime build target, but that can
be overridden on the command line in any case.

The port can be built as:
m6811
m68hc11
m6812
m68hc12
m9s12x  (I added this)

The many options here has caused a problem because most of the testcases
were set to only run when built as m6811. Prior to the last release,
Tristan built the port and ran the tests as m68hc11 - the tests passed
because most/all of the them were in fact ignored. Had the tests run, it
would have exposed a number of long standing bugs. 

A week or so back I sent a mail to the previous maintainer, Stephane
Carrez to see if he recalled why, but no reply as of yet. I also posted
on the yahoo group for gnu hc11/12 but no replies there either.


*) You have included patches to various ChangeLogs.  Common practice is 
> just to include changelog entries as plain text, since they almost
never 
> apply cleanly as patches.
> *) You have added new options to the m68hc11 GAS port, but not added 
> documentation for these new options to the gas/doc/c-m68hc11.texi
file.
> 
> *) You have added support for a new processor, but not mentioned it
in 
> either gas/NEWS or ld/NEWS.

OK, will do. I'll take a look at other submissions as examples.

*) There are some formatting problems.  Ideally we like code that 
> follows the GNU Coding Standard:
 
OK, will do.

*) There appears to be some kind of problem with assembling or 
> disassembling NOP insns for the xgate target.  I tried this:
> 
>    % cat fred.s
>          .text
>    fred:
>          nop
> 
>    % as -mm9s12xg fred.s -o fred.o
> 
>    % objdump -d fred.o
> 
>    fred.o:     file format elf32-m68hc12
> 
>    Disassembly of section .text:
> 
>    00000000 <fred>:
>       0:   01              mem
>            ...
> 
> Assembling with -mm9s12x gives a disassembly of "nop" as expected...
> 

Objdump requires a target option because the ASM is slightly different
between hc11,hc12,9s12x and totally different for 9s12xg (XGATE)

[jsm@jsm2 tmp]$ m9s12x-elf-objdump -mm9s12xg -d fred.o

fred.o:     file format elf32-m9s12xg

Disassembly of section .text:

00000000 <fred>:
   0:	01 00       	nop
[jsm@jsm2 tmp]$ 

Hopefully the new testcases I added demonstrate this too.

I'll see if I can get a revised patch back later this week.

regards

James Murray