68hc11/12/s12x/xgate patch
nick clifton
nickc@redhat.com
Tue Jan 10 14:42:00 GMT 2012
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
More information about the Binutils
mailing list