This is the mail archive of the binutils@sourceware.org mailing list for the binutils project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH 3/5] New target port: Andes 'nds32'. (gas)


2013/7/10 Joseph S. Myers <joseph@codesourcery.com>:
> 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.
-- 
Best regards,
Kuan-Lin Chen.
kuanlinchentw@gmail.com


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]