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

Put each ICACHE_RAM_ATTR function in its own section. #2334

Merged
merged 2 commits into from
Apr 7, 2018

Conversation

pjsg
Copy link
Member

@pjsg pjsg commented Mar 31, 2018

Fixes #2304.

  • This PR is for the dev branch rather than for master.
  • This PR is compliant with the other contributing guidelines as well (if not, please describe why).
  • I have thoroughly tested my contribution.

This implements putting each ICACHE_RAM_ATTR function in its own section. This allows the linker to throw away more functions and hence use less ram.

I checked the map and it looks good. The image boots and seems to work.

@@ -52,7 +52,9 @@ extern void luaL_assertfail(const char *file, int line, const char *message);

#define ICACHE_STORE_TYPEDEF_ATTR __attribute__((aligned(4),packed))
#define ICACHE_STORE_ATTR __attribute__((aligned(4)))
#define ICACHE_RAM_ATTR __attribute__((section(".iram0.text")))
#define STRINGIZE(x) STRINGIZE2(x)
Copy link
Collaborator

@TerryE TerryE Mar 31, 2018

Choose a reason for hiding this comment

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

Defines in user_confgi.h in practice end up getting included in most modules and platform files, so should these be prefixed to remove the risk of a collision with another use of STRINGIZE, e.g.

  • STRINGIZE->ICACHE_STRINGIZE
  • STRINGIZE2->ICACHE_STRINGIZE2

Other than that, 👍

@pjsg
Copy link
Member Author

pjsg commented Mar 31, 2018

It turns out that the hw_timer is included by default as it is needed by the gpio.serout functionality that is part of the default build....

I fixed the macro names...

Copy link
Member

@devsaurus devsaurus left a comment

Choose a reason for hiding this comment

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

Looks good 👍

@marcelstoer marcelstoer added this to the 2.2-follow-up milestone Apr 5, 2018
@devsaurus devsaurus merged commit cda8da6 into nodemcu:dev Apr 7, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants