Use include linker scripts#2841
Conversation
|
The files have been split up as follows:
The linker scripts have been split up into these separate files:
|
|
Would this change also simplify the linker-script modifications in #2840 ? |
Yes, it would mean that PR modifies fewer files |
Allows for much simpler custom linker scripts
…ript_var variables Means that CMake doesn't need to know the default memory addresses for different platforms
…l files easier Restructured so that it includes the platform-specific files before common ones, so common ones can be overridden
Use new include_linker_script_dir and use_linker_script_file functions to add the linker arguments
Breaking change for Bazel builds using different binary types, instead of setting PICO_DEFAULT_LINKER_SCRIPT to eg `//src/rp2_common/pico_crt0:no_flash_linker_script` it is now `//src/rp2_common/pico_standard_link:no_flash_linker_script`
Treat rp2040 layout (boot2 instead of embedded blocks) as the outlier
will-v-pi
force-pushed
the
include-linker-scripts
branch
from
afd45f4 to
9c0ed0c
Compare
Allows overriding what to exclude from .text/.rodata and put in .data
will-v-pi
force-pushed
the
include-linker-scripts
branch
from
8c4476c to
51b433d
Compare
|
An example of using this PR to replace the MicroPython linker scripts: micropython/micropython@master...will-v-pi:micropython:include-linker-scripts This replaces the whole copied linker script (which for RP2040 was still mostly pre-2.0.0), by only overriding:
|
src/rp2_common/pico_standard_link/script_include/section_end.incl
Outdated
Show resolved
Hide resolved
yup, this is all hard! |
|
Intended for platform specific overrides, whereas post_end is for cross-platform overrides, similar to section_end vs section_platform_end
src/rp2_common/pico_standard_link/script_include/section_no_flash_text.incl
Show resolved
Hide resolved
|
@kilograham I think I have made all of those changes now, and also added empty files for easy overriding, so I think this is ready for re-review |
Existing method of just setting the linker script didn't work, because the PICO_NO_FLASH/PICO_COPY_TO_RAM define was only available in pico_platform, but needed to be propogated to things that only use pico_base_headers
|
In doing this PR, I've noticed that setting This was causing compilation failure on no_flash because I've fixed that in this PR by implementing a CC @armandomontanez in case my understanding of this is incorrect |
| endfunction() | ||
|
|
||
| # PICO_CMAKE_CONFIG: PICO_DEFAULT_BINARY_TYPE, The default binary type to use, type=string, default=default, group=pico_standard_link | ||
| # PICO_CMAKE_CONFIG: PICO_DEFAULT_BINARY_TYPE, The default binary type to use, type=string, default=default, group=build |
There was a problem hiding this comment.
Is it worth clarifying what "default" means, or will it resolve to different things in different scenarios?
There was a problem hiding this comment.
It's annoying naming, but should be maintained for consistency - the "default" binary type is a flash binary, as opposed to "no_flash" which is an SRAM-only binary, and "copy_to_ram" which is a flash binary that copies all it's code to SRAM before executing it
armandomontanez
left a comment
armandomontanez
left a comment
There was a problem hiding this comment.
In doing this PR, I've noticed that setting
PICO_DEFAULT_LINKER_SCRIPTto use the no_flash and copy_to_ram linker scripts in Bazel was not working, due toPICO_NO_FLASH/PICO_COPY_TO_RAMonly being set onpico_platformbut being required by some things that only depend onpico_platform_internalorpico_base_headers(egpico_crt0andpico_standard_binary_info).
That makes sense, I remember there's a pretty gnarly circular dependency that's difficult to unwind (hence the _internal bits). The workaround you put together looks fine.
| ]), | ||
| ) | ||
|
|
||
| include_linker_script_dir( |
There was a problem hiding this comment.
I'd pitch an interface like this:
linker_script(
name = "default_locations",
library_paths = ["."],
additional_linker_inputs = glob(["*.incl"]),
srcs = [
"default_locations.ld",
],
)
If that's too much of a pain to implement, feel free to punt and leave me an action item to clean it up.
There was a problem hiding this comment.
Thanks for all your comments - I've changed it to this interface:
linker_scripts(
name = "rp2040_linker_script_includes",
link_scripts = ["default_locations.ld"],
include_scripts = glob(["*.incl"]),
)
I think this makes it clearer - the link_scripts are .ld files explicitly linked with -T, whereas include_scripts are .incl files which have their package directory included with -L and are added as additional inputs
I couldn't see how to implement library_paths = ["."], so instead opted for getting the paths from the .label.package of the include_scripts, which I think is good solution
…e for individual binaries Can remove full no_flash etc test builds, as there can now be kitchen_sink_no_flash etc builds like in CMake
Now uses single linker_scripts function to allow both including and linking scripts Returns DefaultInfo with all necessary files Prepends ctx.label.workspace_root to link_include_dir Gets link_include_dir directly from the include_scripts passed in
… bazel Also adds pico_set_linker_script transition to set linker script for individual binaries
|
note, it would be nice (and i toyed briefly locally) with putting some directory structure in here, but i guess you would have done that already if the linker did the right thing to allow overrides for say |
|
perhaps the nicest thing would be to just draw a tree in a README.md for all the default linker script types ([erhaps at the platform level) e.g. but with full tree and better ASCII corners! Note i don'y know if it is worth highlighting any directives but maybe ENTRY_POINT is the only one |
|
^ I guess that could go in a comment in the top-level makefile in platforms/ *we could certainly write a quick tool to generate |
|
that could be a separate PR though to unblock other stuff |
I wasn't doing this because then users have to create the correct directory structure in the override directories rather than just needing the correct filename. There are also lots of common files shared between 2 or 3 binary types (eg But it would probably work to put them in folders if you think that would be better |
Remove whitespace, and rename memory_aliases_default to memory_aliases_flash, as it applies to copy_to_ram too
|
Note I would actually write a script that does this as a comment in all the scripts and incl files (and lists direct parents) |
Something like this output? |
Co-authored-by: Andrew Scheller <lurch@durge.org>
Though since it is script generated let’s align the columns… put something like rp2_common/pico_standard_link as the second column |
|
Also wrt the trees - let’s |
4 participants
This should make it easier to modify linker scripts, without needing to copy the entire script, as you can only modify the parts you need to and use the SDK defaults for the rest.
For example, if you want a linker script using only a specific region of RAM, the custom linker script would be:
This could also be achieved using CMake functions:
For more complexity, you can also override specific files in the linker script by creating an override directory:
then any files placed in that directory will override the defaults - eg to add a custom section after data before heap, you would add a
section_extra_post_data.inclfile:or to add an overlay section (kitchen_sink_simple_overlay) you would use:
Both of these would override the default empty
section_extra_post_data.inclfile