This is the mail archive of the
binutils@sourceware.org
mailing list for the binutils project.
Re: 68hc11/12/s12x/xgate patch
- From: nick clifton <nickc at redhat dot com>
- To: James Murray <jsm at jsm-net dot demon dot co dot uk>
- Cc: binutils at sourceware dot org
- Date: Tue, 10 Jan 2012 14:41:02 +0000
- Subject: Re: 68hc11/12/s12x/xgate patch
- References: <1298845471.12108.12.camel@jsm2> <1299515895.3262.1.camel@jsm2> <4D7F247D.6070303@redhat.com> <1300201861.20997.3.camel@jsm2> <1324419607.8652.84.camel@jsm2> <CABZhLO-Ye-G1nYW1eqpdjnYG1W4zaKq+TvgzFBebJk4ash5mtg@mail.gmail.com> <1324423375.8652.94.camel@jsm2> <4EF36049.3050802@ipdatasys.com> <CABZhLO_RKXkcKRGH_b-f_iVYppcgBQ3XBf9STWjZeTCwrQJhzQ@mail.gmail.com> <1324575421.2430.42.camel@jsm2> <1325800936.20629.53.camel@jsm2>
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 ?
*) 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)
*) 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.
*) There are some formatting problems. Ideally we like code that
follows the GNU Coding Standard: http://www.gnu.org/prep/standards/
Problems such as lines that end with opening curly braces:
if (r_type != R_M68HC11_NONE) {
Comments that are not treated as sentences:
/* the -2 here is due to the way XGATE PCREL9, PCREL10 work */
Lines that are excessively long and could be broken up:
if (move_insn && !(val >= -16 && val <= 15) && ((!(mode &
M6812_OP_IDX) && !(mode & M6812_OP_D_IDX_2)) || !(current_architecture &
cpu9s12x)))
Function calls without a space between the function name and the opening
parentheses.
as_bad("Invalid register\n");
*) 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...
Other than that though, this patch looks fine. If you could address the
issues raised above and resubmit the patch I will be happy to review it
again.
Cheers
Nick