Re: [PATCH 2/6] Andes nds32: machine description of nds32 porting (1).

[Chung-Ju Wu <jasonwucj at gmail dot com>]

On 7/25/13 12:08 AM, Joseph S. Myers wrote:
> On Wed, 24 Jul 2013, Chung-Ju Wu wrote:
>
> > We removed the fancy comment formatting as you suggested.
> > However, for the comments like:
> >
> > /* ===================== */
> > /* Title of section */
> > /* ===================== */
> >
> > /* Title of subsection 1 */
> >
> > /* Title of subsection 2 */
> >
> > I wish I can keep such kinds of comment formatting so that
> > it would be easier to distinguish the levels between sections
> > and subsections against the chapter 17 in GCC Internals documentation.
> >
> > Is it ok to keep such formatting? :)
>
> You can use form-feeds (control-L characters) to separate logical sections
> in a source file.

Oh! Good point!  Thanks for the tip.
We modified it accordingly. :)

> > > > +/* Define intrinsic register names.
> > > > +   Please refer to nds32_intrinsic.h file, the index is corresponding to
> > > > +   'enum nds32_intrinsic_registers' data type values.
> > > > +   NOTE that the base value starting from 1024.  */
> > > > +static const char* nds32_intrinsic_register_names[] =
> > >
> > > I suspect the array here can itself be made const.
> >
> > I am declaring an array in which every element is "const char *" type.
> > It is different to the declaration:
> > "static char* const nds32_intrinsic_register_names[] ="
>
> The question is whether it can be "static const char *const" - or do you
> need to modify the array itself at runtime (which would be unfortunate in
> terms of the aim to avoid global state)?

Ah... now I get it.
You are correct, it can be declared as "static const char* const".
The array itself is not modified at runtime in our porting.

Sorry for that I misunderstood your words previously.

Attached is another revised patch and its summary.
The modifications described above are listed as item 11 and 12:

   1. Remove fancy comments formatting.
   2. Sort static functions and variables and put the target hook
      structure initialization at the end of the file.
   3. Avoid all comments referring to section numbers.
   4. Use snprintf () instead of sprinf ().
   5. Use sorry () instead of error () for unimplemented functionalities.
   6. Do not check command options error in nds32.h.
     Use Negative() in nds32.opt instead.
   7. Remove %{v} from LINK_SPEC.
   8. Add OPTION_DEFAULT_SPECS for -march option.
   9. Enable/disable corresponding flags internally for -march= option.
  10. Add MULTILIB_DEFAULTS for multilib endianness.
  11. Use form-feeds (Control-L character) to separate logical sections.
  12. The nds32_intrinsic_register_names[] array itself can be made const.

Note that for item 11, we need to make changes on other source files as well.
So we also create other revised patches for
"[PATCH 2/6] Andes nds32: machine description of nds32 porting (2)"
and "[PATCH 2/6] Andes nds32: machine description of nds32 porting (3)".

Thanks again for your review comments.


Best regards,
jasonwucj

Attachment: 2-nds32-backend-md-part1.v2.patch
Description: Text document