Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

#11791: linker script cleanups #13305

Merged
merged 2 commits into from
Oct 2, 2024
Merged

#11791: linker script cleanups #13305

merged 2 commits into from
Oct 2, 2024

Conversation

nathan-TT
Copy link
Contributor

@nathan-TT nathan-TT commented Sep 30, 2024

Ticket

#11791

Problem description

I need to alter the linker scripts for XIP support - make sure PHDRS are as expected. But I noticed a few things that are a bit askew anyway, and these got exacerbated by @pgkeller 's recent changes

  1. Rather than generate additional symbols, use more #define and let the preprocessor expand as needed
  2. Now we generate separate fw and kernel scripts, we don't need the confusingly subtle foo = defined(foo) ? foo : . idiom. Just define the appropriate value
  3. A bunch of symbols -- __executable_start, _etext, end, _end etc make sense for traditional 2-segment unix executables that may want to examine themselves and support older source code using legacy symbol names. None of that is true here, let's just get rid of that.
  4. The equating of _Z11kernel_initv = _etext; is particularly egregious -- it only makes sense in FW, and it's presuming _etext is where the kernel starts -- just get the script to assign the right value itself.
  5. The comment about needing an initializer on local_data_noinit is correct as far as it goes, but let's just force the first data section to be emitted by assigning to . in it.
  6. And while there remove and/or clean up some other assignments to .

What's changed

As above, no change in used functionality

Checklist

  • [YES] Post commit CI passes
  • Blackhole Post commit (if applicable)
  • [NA] Model regression CI testing passes (if applicable)
  • [NA] Device performance regression CI testing passes (if applicable)
  • [YES] New/Existing tests provide coverage for changes

Copy link
Contributor

@pgkeller pgkeller left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is great cleanup. I'd like #0 to only be for trivial changes (formatting, etc), this should be attributed to an issue. Since it is work towards 11791, could use that

@nathan-TT nathan-TT changed the title #0: linker script cleanups #11791: linker script cleanups Oct 1, 2024
@nathan-TT
Copy link
Contributor Author

this is great cleanup. I'd like #0 to only be for trivial changes (formatting, etc), this should be attributed to an issue. Since it is work towards 11791, could use that
done

@nathan-TT nathan-TT merged commit 1831173 into main Oct 2, 2024
6 checks passed
@nathan-TT nathan-TT deleted the nsidwell/linker2 branch October 2, 2024 14:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants