This is the mail archive of the
mailing list for the binutils project.
Re: [PATCH 3/5] New target port: Andes 'nds32'. (gas)
- From: Kuan-Lin Chen <kuanlinchentw at gmail dot com>
- To: "Joseph S. Myers" <joseph at codesourcery dot com>
- Cc: binutils at sourceware dot org
- Date: Thu, 11 Jul 2013 10:36:37 +0800
- Subject: Re: [PATCH 3/5] New target port: Andes 'nds32'. (gas)
- References: <CAJr6u0heVGU2kYOWh_5TR2b+x=sMOGH0xfCc3y5LCGA3tqCqEQ at mail dot gmail dot com> <Pine dot LNX dot 4 dot 64 dot 1307091641230 dot 23421 at digraph dot polyomino dot org dot uk> <CAJr6u0gNSkY++5EUBzudg2x_+Vq8hvKheRdLddJeL+rvM5Kyfw at mail dot gmail dot com>
2013/7/10 Joseph S. Myers <firstname.lastname@example.org>:
> This patch has several FIXME / TODO / "Review" comments, which should be
> reviewed and, as appropriate, fixed. There are also "COLE" comments; I
> don't know what that means, but comments should be readily intelligible to
> anyone reading the sources, so I advise avoiding such codewords.
I am trying to review and fix all of them. BTW, "COLE" is the name of my
team member, and I will remove all the name in the comment.
> You seem to have a lot of conditionals in tc-nds32.c for changing the
> defaults of particular features - but nothing in configure.in that would
> actually allow anyone to use those conditionals. I think such
> conditionals are a bad idea and should be removed; just make GCC pass
> appropriate configuration to the assembler, whether by command-line
> options, directives in the .s file, or both.
Since our target support lots of different platform, some features are relative
to target board. May I modify it like xtensa (include/xtensa-config.h) using the
header file to control these features.
> As has already been noted, this patch is missing documentation for the
> port. When documenting command-line options for the assembler, see
> <http://sourceware.org/ml/binutils/2010-11/msg00397.html> for instructions
> on how to get the documentation in all the right places without needing to
> duplicate it.
I will send the documentation patch when all of the directory gas/ is done.
Thanks for your review.