[PATCH RFC v2] Add support for non-contiguous memory regions

classic Classic list List threaded Threaded
12 messages Options
Reply | Threaded
Open this post in threaded view
|

[PATCH RFC v2] Add support for non-contiguous memory regions

Christophe Lyon-2
Hi,

This is a follow-up to
https://sourceware.org/ml/binutils/2019-11/msg00402.html

This version detects cases it isn't (yet?) able to handle and emits an
error message (and uses abort() which can probably be made more
user-friendly).

Specifically, I have:
* improved the testcase in ld-elf, hoping that it's now generic enough
(at least it passes on arm and powerpc).

* added a powerpc testcase, based on the sample provided by Simon. It
now fails with an error message when relaxation was applied.

* added several arm testcases, some of which involve farcall stubs
insertion. Something annoying with stubs handling is that I had to
modify arm-specific code to generate an error in a case where the stub
destination could not be allocated. We'd have to do this in every
target that has XXX_build_one_stub().
I now add the SEC_LINKER_CREATED flag on the stubs sections so that we
can emit a helpful error message in case the stubs do not fit in
memory.

There's a new test to check that section order does not change.

Finally, I had to change the way I remove input_section statements in
lang_size_sections_1() to correctly handle the case where we have to
remove the head of the list.

There are now 4 patches, to hopefully make review/comments easier:
* patch1: is the main (code) patch
* patch2: generic test
* patch3: arm tests
* patch4: powerpc test

Thoughts?

Thanks,

Christophe

0003-Add-arm-tests-for-non-contiguous-memory-regions.patch (31K) Download Attachment
0002-Add-test-for-non-contiguous-memory-regions.patch (5K) Download Attachment
0004-Add-powerpc-test-for-non-contiguous-memory-regions.patch (3K) Download Attachment
0001-Add-support-for-non-contiguous-memory-regions.patch (16K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH RFC v2] Add support for non-contiguous memory regions

Nick Clifton
Hi Christophe,

> There are now 4 patches, to hopefully make review/comments easier:
> * patch1: is the main (code) patch
> * patch2: generic test
> * patch3: arm tests
> * patch4: powerpc test

I think that you need to test some non-ELF based targets.  Or maybe a
--enable-targets=all configuration.  I am seeing this error when building:

  bfd/ecoff.c:81:1: error: missing initializer for field 'already_assigned' of 'asection' {aka 'struct bfd_section'} [-Werror=missing-field-initializers]

Also when adding a new feature like this, it would be nice if you could
include a line or two in ld/NEWS describing what it does.  This makes it
easier for me to advertise the feature when announcing a new release...

Also you should add a description of the option to ld/ld.texi.

Cheers
  Nick


Reply | Threaded
Open this post in threaded view
|

Re: [PATCH RFC v2] Add support for non-contiguous memory regions

Christophe Lyon-2
On Wed, 8 Jan 2020 at 14:14, Nick Clifton <[hidden email]> wrote:

>
> Hi Christophe,
>
> > There are now 4 patches, to hopefully make review/comments easier:
> > * patch1: is the main (code) patch
> > * patch2: generic test
> > * patch3: arm tests
> > * patch4: powerpc test
>
> I think that you need to test some non-ELF based targets.  Or maybe a
> --enable-targets=all configuration.  I am seeing this error when building:
>
>   bfd/ecoff.c:81:1: error: missing initializer for field 'already_assigned' of 'asection' {aka 'struct bfd_section'} [-Werror=missing-field-initializers]

Right, I didn't try non-elf targets.
If it helps your testing, I was just missing:
diff --git a/bfd/ecoff.c b/bfd/ecoff.c
index be3d42e..2ce2c8f 100644
--- a/bfd/ecoff.c
+++ b/bfd/ecoff.c
@@ -77,7 +77,9 @@ static asection bfd_debug_section =
   /* symbol_ptr_ptr,                                              */
      NULL,
   /* map_head, map_tail                                                   */
-     { NULL }, { NULL }
+     { NULL }, { NULL },
+  /* already_assigned                                             */
+     NULL,
 };

 /* Create an ECOFF object.  */

>
> Also when adding a new feature like this, it would be nice if you could
> include a line or two in ld/NEWS describing what it does.  This makes it
> easier for me to advertise the feature when announcing a new release...
>
> Also you should add a description of the option to ld/ld.texi.

Sure, I am aware of that. At this stage, I'm still looking for comments
on the design. I fear I'm doing changes in a very wrong way.
OTOH, if this approach looks fine to you, I'll make sure to submit a patch
including some docs before the 2.34 deadline :-)

Thanks,

Christophe

>
> Cheers
>   Nick
>
>
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH RFC v2] Add support for non-contiguous memory regions

Christophe Lyon-2
Hi,


On Wed, 8 Jan 2020 at 14:59, Christophe Lyon <[hidden email]> wrote:

>
> On Wed, 8 Jan 2020 at 14:14, Nick Clifton <[hidden email]> wrote:
> >
> > Hi Christophe,
> >
> > > There are now 4 patches, to hopefully make review/comments easier:
> > > * patch1: is the main (code) patch
> > > * patch2: generic test
> > > * patch3: arm tests
> > > * patch4: powerpc test
> >
> > I think that you need to test some non-ELF based targets.  Or maybe a
> > --enable-targets=all configuration.  I am seeing this error when building:
> >
> >   bfd/ecoff.c:81:1: error: missing initializer for field 'already_assigned' of 'asection' {aka 'struct bfd_section'} [-Werror=missing-field-initializers]
>
> Right, I didn't try non-elf targets.
> If it helps your testing, I was just missing:
> diff --git a/bfd/ecoff.c b/bfd/ecoff.c
> index be3d42e..2ce2c8f 100644
> --- a/bfd/ecoff.c
> +++ b/bfd/ecoff.c
> @@ -77,7 +77,9 @@ static asection bfd_debug_section =
>    /* symbol_ptr_ptr,                                              */
>       NULL,
>    /* map_head, map_tail                                                   */
> -     { NULL }, { NULL }
> +     { NULL }, { NULL },
> +  /* already_assigned                                             */
> +     NULL,
>  };
>
>  /* Create an ECOFF object.  */
>
> >
> > Also when adding a new feature like this, it would be nice if you could
> > include a line or two in ld/NEWS describing what it does.  This makes it
> > easier for me to advertise the feature when announcing a new release...
> >
> > Also you should add a description of the option to ld/ld.texi.
>
> Sure, I am aware of that. At this stage, I'm still looking for comments
> on the design. I fear I'm doing changes in a very wrong way.
> OTOH, if this approach looks fine to you, I'll make sure to submit a patch
> including some docs before the 2.34 deadline :-)
>
Here is an updated version:
* fixes non-elf build
* adds entries in NEWS and ld.texi

I'm wondering whether I should do something cleaner than abort() when
detecting an unsupported case?

Also, I'm still not sure about the new option name....

Since it changes the processing to allow several output sections to
match, maybe --enable-non-contiguous-regions is not good although it
describes the original need.
Would --multiple-output-sections be better? (actually, there's still
only one output in the end....)

Thanks,

Christophe

> Thanks,
>
> Christophe
>
> >
> > Cheers
> >   Nick
> >
> >

0001-Add-support-for-non-contiguous-memory-regions.patch (19K) Download Attachment
0002-Add-generic-test-for-non-contiguous-memory-regions.patch (5K) Download Attachment
0004-Add-powerpc-test-for-non-contiguous-memory-regions.patch (4K) Download Attachment
0003-Add-arm-tests-for-non-contiguous-memory-regions.patch (30K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH RFC v2] Add support for non-contiguous memory regions

Simon Richter-2
Hi,

On Fri, Jan 10, 2020 at 11:22:31AM +0100, Christophe Lyon wrote:

> Since it changes the processing to allow several output sections to
> match, maybe --enable-non-contiguous-regions is not good although it
> describes the original need.
> Would --multiple-output-sections be better? (actually, there's still
> only one output in the end....)

Wait, this option means that if an input section is listed multiple times,
it isn't duplicated like it used to be?

That might be a problem for people who are building overlays for banked
memory, or to conserve RAM.

   Simon
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH RFC v2] Add support for non-contiguous memory regions

Christophe Lyon-2
On Fri, 10 Jan 2020 at 11:42, Simon Richter <[hidden email]> wrote:

>
> Hi,
>
> On Fri, Jan 10, 2020 at 11:22:31AM +0100, Christophe Lyon wrote:
>
> > Since it changes the processing to allow several output sections to
> > match, maybe --enable-non-contiguous-regions is not good although it
> > describes the original need.
> > Would --multiple-output-sections be better? (actually, there's still
> > only one output in the end....)
>
> Wait, this option means that if an input section is listed multiple times,
> it isn't duplicated like it used to be?
>
> That might be a problem for people who are building overlays for banked
> memory, or to conserve RAM.
>

I haven't checked that. Is there an existing testcase?

>    Simon
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH RFC v2] Add support for non-contiguous memory regions

Christophe Lyon-2
On Fri, 10 Jan 2020 at 12:02, Christophe Lyon
<[hidden email]> wrote:

>
> On Fri, 10 Jan 2020 at 11:42, Simon Richter <[hidden email]> wrote:
> >
> > Hi,
> >
> > On Fri, Jan 10, 2020 at 11:22:31AM +0100, Christophe Lyon wrote:
> >
> > > Since it changes the processing to allow several output sections to
> > > match, maybe --enable-non-contiguous-regions is not good although it
> > > describes the original need.
> > > Would --multiple-output-sections be better? (actually, there's still
> > > only one output in the end....)
> >
> > Wait, this option means that if an input section is listed multiple times,
> > it isn't duplicated like it used to be?
> >
> > That might be a problem for people who are building overlays for banked
> > memory, or to conserve RAM.
> >
>
> I haven't checked that. Is there an existing testcase?
>
I should probably have phrased that differently: I ran the testsuite
for arm and powerpc, no regression.
Do you have a case in mind that is not covered by the existing testsuite?


> >    Simon
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH RFC v2] Add support for non-contiguous memory regions

Simon Richter-2
Hi,

sorry, overlooked the original email.

> > > Wait, this option means that if an input section is listed multiple times,
> > > it isn't duplicated like it used to be?

> > > That might be a problem for people who are building overlays for banked
> > > memory, or to conserve RAM.

> > I haven't checked that. Is there an existing testcase?

> I should probably have phrased that differently: I ran the testsuite
> for arm and powerpc, no regression.
> Do you have a case in mind that is not covered by the existing testsuite?

Did you run the testsuite with --enable-non-contiguous-regions? If so, that
looks like a gap in test coverage to me, e.g. I'd fully expect to be able
to do something like

trampoline.S:

        .section .text.trampoline
        stmdb sp, {r10}
        ldr r10, =BANK_SWITCH
        str r11, [r10]
        ldmia sp, {r10}
        nop
        nop
        nop
        nop
        nop
        b r11

and then reuse that section on every bank:

        SECTIONS {
                .bank0 0x0 : {
                        *(.text.trampoline)
                        *(.text.bank0)
                } >BANK0
                .bank1 0x10000 : {
                        *(.text.trampoline)
                        *(.text.bank1)
                } >BANK1
        }

in order to generate binary images that contain the repeated parts for all
memory banks.

The official method

        SECTIONS {
                .text.trampoline : {
                        *(.text.trampoline)
                }
                OVERLAY : {
                        .bank0 : {
                                *(.text.bank0)
                        }
                        .bank1 : {
                                *(.text.bank1)
                        }
                }
        }

is a lot less useful for generating images ready for programming, because
it assumes that we are building a single image that a loader will pull into
RAM piecewise.

I'm certain there are other use cases for repeated sections as well, bank
switching is more of a thing on smaller CPUs like the 8051, which have
interrupt trampolines on every bank because the interrupt handling logic
does not know about the memory map[1].

As usual, I'm only looking for possible problems here, with an emphasis on
"possible" -- whether these are relevant from a project management POV is
up to the actual project maintainers.

As a user, it'd be awesome to be able to use the combination of
non-contiguous regions, banking with a trampoline and linker relaxations to
automatically distribute code over multiple banks and generate the most
efficient jump instructions between them, but that would be a rather
monumental task.

   Simon

[1] http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.faqs/ka8894.html
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH RFC v2] Add support for non-contiguous memory regions

Christophe Lyon-2
On Mon, 13 Jan 2020 at 15:37, Simon Richter <[hidden email]> wrote:

>
> Hi,
>
> sorry, overlooked the original email.
>
> > > > Wait, this option means that if an input section is listed multiple times,
> > > > it isn't duplicated like it used to be?
>
> > > > That might be a problem for people who are building overlays for banked
> > > > memory, or to conserve RAM.
>
> > > I haven't checked that. Is there an existing testcase?
>
> > I should probably have phrased that differently: I ran the testsuite
> > for arm and powerpc, no regression.
> > Do you have a case in mind that is not covered by the existing testsuite?
>
> Did you run the testsuite with --enable-non-contiguous-regions? If so, that
Yes, but after I sent my previous email.... and it shows a few
regressions, which I'm looking at.

However the *overlay* tests under ld-elf still work.

> looks like a gap in test coverage to me, e.g. I'd fully expect to be able
> to do something like
>
> trampoline.S:
>
>         .section .text.trampoline
>         stmdb sp, {r10}
>         ldr r10, =BANK_SWITCH
>         str r11, [r10]
>         ldmia sp, {r10}
>         nop
>         nop
>         nop
>         nop
>         nop
>         b r11
>
> and then reuse that section on every bank:
>
>         SECTIONS {
>                 .bank0 0x0 : {
>                         *(.text.trampoline)
>                         *(.text.bank0)
>                 } >BANK0
>                 .bank1 0x10000 : {
>                         *(.text.trampoline)
>                         *(.text.bank1)
>                 } >BANK1
>         }
>
> in order to generate binary images that contain the repeated parts for all
> memory banks.
>
> The official method
>
>         SECTIONS {
>                 .text.trampoline : {
>                         *(.text.trampoline)
>                 }
>                 OVERLAY : {
>                         .bank0 : {
>                                 *(.text.bank0)
>                         }
>                         .bank1 : {
>                                 *(.text.bank1)
>                         }
>                 }
>         }
>
> is a lot less useful for generating images ready for programming, because
> it assumes that we are building a single image that a loader will pull into
> RAM piecewise.
>
> I'm certain there are other use cases for repeated sections as well, bank
> switching is more of a thing on smaller CPUs like the 8051, which have
> interrupt trampolines on every bank because the interrupt handling logic
> does not know about the memory map[1].
>
> As usual, I'm only looking for possible problems here, with an emphasis on
> "possible" -- whether these are relevant from a project management POV is
> up to the actual project maintainers.
>
> As a user, it'd be awesome to be able to use the combination of
> non-contiguous regions, banking with a trampoline and linker relaxations to
> automatically distribute code over multiple banks and generate the most
> efficient jump instructions between them, but that would be a rather
> monumental task.
>
>    Simon
>
> [1] http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.faqs/ka8894.html
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH RFC v2] Add support for non-contiguous memory regions

Simon Richter-2
Hi,

On Mon, Jan 13, 2020 at 05:24:07PM +0100, Christophe Lyon wrote:

> > Did you run the testsuite with --enable-non-contiguous-regions? If so, that

> Yes, but after I sent my previous email.... and it shows a few
> regressions, which I'm looking at.

> However the *overlay* tests under ld-elf still work.

Yes, the way overlays are implemented it should be unaffected, because
input sections aren't used twice in that setup.

The bank switching trampoline example is one case where a user might want
to have input sections duplicated during linking -- there might be others.

Unrelated to your patch, that might be an interesting use case as well:
statically linking libraries into overlays:

library.S:

                .section .text
                .globl function
        function:
                blr

overlay1.S:

                .section .text
                mov r0, #0
                bl function

overlay2.S:

                .section .text
                mov r0, #1
                bl function

test.ld:

        SECTIONS {
                OVERLAY : NOCROSSREFS {
                        .text.overlay1 : {
                                overlay1.o(.text)
                                EXCLUDE_FILE(overlay*.o) *(.text)
                        }
                        .text.overlay2 : {
                                overlay2.o(.text)
                                EXCLUDE_FILE(overlay*.o) *(.text)
                        }
                }
        }

My expectation here would be that I'd end up with two copies of "function",
one in each branch of the overlay.

   Simon
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH RFC v2] Add support for non-contiguous memory regions

Christophe Lyon-2
In reply to this post by Christophe Lyon-2
On Mon, 13 Jan 2020 at 17:24, Christophe Lyon
<[hidden email]> wrote:

>
> On Mon, 13 Jan 2020 at 15:37, Simon Richter <[hidden email]> wrote:
> >
> > Hi,
> >
> > sorry, overlooked the original email.
> >
> > > > > Wait, this option means that if an input section is listed multiple times,
> > > > > it isn't duplicated like it used to be?
> >
> > > > > That might be a problem for people who are building overlays for banked
> > > > > memory, or to conserve RAM.
> >
> > > > I haven't checked that. Is there an existing testcase?
> >
> > > I should probably have phrased that differently: I ran the testsuite
> > > for arm and powerpc, no regression.
> > > Do you have a case in mind that is not covered by the existing testsuite?
> >
> > Did you run the testsuite with --enable-non-contiguous-regions? If so, that
> Yes, but after I sent my previous email.... and it shows a few
> regressions, which I'm looking at.

I have patches for the problems I noticed (took me a while to realize
that my new option is incompatible with "INSERT", since it can change
the input section -> output section mapping).

>
> However the *overlay* tests under ld-elf still work.
>
> > looks like a gap in test coverage to me, e.g. I'd fully expect to be able
> > to do something like
> >
> > trampoline.S:
> >
> >         .section .text.trampoline
> >         stmdb sp, {r10}
> >         ldr r10, =BANK_SWITCH
> >         str r11, [r10]
> >         ldmia sp, {r10}
> >         nop
> >         nop
> >         nop
> >         nop
> >         nop
> >         b r11
> >
> > and then reuse that section on every bank:
> >
> >         SECTIONS {
> >                 .bank0 0x0 : {
> >                         *(.text.trampoline)
> >                         *(.text.bank0)
> >                 } >BANK0
> >                 .bank1 0x10000 : {
> >                         *(.text.trampoline)
> >                         *(.text.bank1)
> >                 } >BANK1
> >         }
> >
> > in order to generate binary images that contain the repeated parts for all
> > memory banks.
> >

What did you actually mean earlier by "Wait, this option means that if
an input section is listed multiple times,
it isn't duplicated like it used to be?"
In which cases are sections duplicated (when you say "used to be")? My
understanding is that input section have only one output section.

Thanks,

Christophe


> > The official method
> >
> >         SECTIONS {
> >                 .text.trampoline : {
> >                         *(.text.trampoline)
> >                 }
> >                 OVERLAY : {
> >                         .bank0 : {
> >                                 *(.text.bank0)
> >                         }
> >                         .bank1 : {
> >                                 *(.text.bank1)
> >                         }
> >                 }
> >         }
> >
> > is a lot less useful for generating images ready for programming, because
> > it assumes that we are building a single image that a loader will pull into
> > RAM piecewise.
> >
> > I'm certain there are other use cases for repeated sections as well, bank
> > switching is more of a thing on smaller CPUs like the 8051, which have
> > interrupt trampolines on every bank because the interrupt handling logic
> > does not know about the memory map[1].
> >
> > As usual, I'm only looking for possible problems here, with an emphasis on
> > "possible" -- whether these are relevant from a project management POV is
> > up to the actual project maintainers.
> >
> > As a user, it'd be awesome to be able to use the combination of
> > non-contiguous regions, banking with a trampoline and linker relaxations to
> > automatically distribute code over multiple banks and generate the most
> > efficient jump instructions between them, but that would be a rather
> > monumental task.
> >
> >    Simon
> >
> > [1] http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.faqs/ka8894.html
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH RFC v2] Add support for non-contiguous memory regions

Simon Richter-2
Hi,

On 29.01.20 22:27, Christophe Lyon wrote:

> What did you actually mean earlier by "Wait, this option means that if
> an input section is listed multiple times,
> it isn't duplicated like it used to be?"
> In which cases are sections duplicated (when you say "used to be")? My
> understanding is that input section have only one output section.

True, no idea why I thought this would work, but I kind of expected it
to. Sorry for the noise.

   Simon


signature.asc (499 bytes) Download Attachment