This is the mail archive of the 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 0/4][MSP430] Improve behaviour of ".either" section placement

Hi Jozef,

> GCC for the MSP430 target has a feature that allows the user to specify
> which memory region to place code and/or data in, using the -mcode-
> region and -mdata-region options.
> MSP430 devices with a high memory region support this placement of
> code/data by defining ".upper" and ".lower" output sections in linker
> scripts.
> The parameters that can be passed to -mcode/data-region are "none",
> "upper", "lower", "either". The use of the "either" option requires some
> work to be done by the linker, to place ".either" code and data sections
> in either the upper or lower regions to prevent memory region overflow,
> if possible. The following patches improve and fix some of the existing
> behaviour of this ".either" placement functionality.

Thanks very much for doing this.  It is really nice to see someone interested
in developing the MSP430 toolchain.

Rather than comment on each patch individually I have some general comments
that apply to all of the patches:

  * Please could you provide ChangeLog entries for each of the patches.

  * Please move variable declarations to the start of their containing
    blocks.  I know that modern C compilers allow them anywhere in the
    block, but we still need to support some older compilers that do not.

  * Please used ATTRIBUTE_UNUSED instead of __attribute__((unused)).

  * Please format comments according to the GNU coding standard.  
    Specifically a comment like this:
      /* This is a
       * multiline comment.

    Should be formatted as:

     /* This is a
        multiline comment.  */

Also, there is one specific comments on the behaviour of patch series:

  * It seems to me that when performing the last UPPER to LOWER move it
    would be better to move any section that will fit into LOWER, even
    if UPPER is *not* overflowing.  This assumes that LOWER memory is faster
    and hence it is preferable to use as much of it as possible.

But all in all, I think that the patch is great and I am looking forward to 
applying it.


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