-
Notifications
You must be signed in to change notification settings - Fork 3k
NANO130: Fix OOM with packing algorithm at IAR linking #12047
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
Conversation
At IAR linking, the default method of 'initialize by copy' is 'auto', which will estimate different packing algorithms, including complex 'lz77', for smallest memory footprint. But the algorithm itself can consume some SRAM and cause OOM at linking time for NANO130, which just has 16KiB SRAM. To avoid this error, always choose 'none' packing algorithm.
@ccli8, thank you for your changes. |
This adjustment is to pass compile on IAR on which heap configuration is hard-coded and cannot be adjusted according to staic SRAM usage.
989888f
to
3e4e062
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We haven't yet enabled dynamic, have we ? 👀 cc @mprse |
As there is a dependancy on this one for 5.15rc2, this one was set to the same version 5.15rc2 cc @adbridge |
@ccli8 see https://github.com/ARMmbed/mbed-os/pull/10938/files , can this target one as well? Change line 24:
|
@@ -24,7 +24,7 @@ define block CSTACK with alignment = 8, size = __ICFEDIT_size_cstack__ { }; | |||
define block HEAP with alignment = 8, size = __ICFEDIT_size_heap__ { }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
define block HEAP with alignment = 8, size = __ICFEDIT_size_heap__ { }; | |
define block HEAP with expanding size, alignment = 8, minimum size = __ICFEDIT_size_heap__ { }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can't commit to the branch, this should do the trick
Dynamic heap change can be done separately, the PR as it is should fix the issue (heap will be set as minimum once we get dynamic heap there) and unblock other PRs . Starting CI now |
Test run: SUCCESSSummary: 11 of 11 test jobs passed |
Summary of changes
This PR tries to address #12033 (comment) for NUMAKER_PFM_NANO130 target:
initialize by copy
isauto
, which will estimate different packing algorithms, including complexlz77
, for smallest memory footprint. But the algorithm itself can require some SRAM overhead and cause OOM at linking stage for small SRAM target like NANO130, which just has 16KiB SRAM. To avoid this error on NANO130, always choosenone
packing algorithm.Pull request type
Test results
Reviewers
@mprse @0xc0170