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)


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.

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.

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.

-- 
Joseph S. Myers
joseph@codesourcery.com


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