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

Move cont_* funcs from ram to flash #4384

Merged
merged 4 commits into from
Mar 31, 2018

Conversation

kylefleming
Copy link
Contributor

I think these can go in flash and not take up ram space since the functions that call them are all also located in flash.

@@ -57,7 +57,7 @@ int ICACHE_RAM_ATTR cont_get_free_stack(cont_t* cont) {
return freeWords * 4;
}

bool ICACHE_RAM_ATTR cont_can_yield(cont_t* cont) {
bool cont_can_yield(cont_t* cont) {
Copy link
Member

Choose a reason for hiding this comment

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

This can be called from ISR, when cache is disabled, so it's best to keep it in IRAM.

@@ -38,13 +38,13 @@ void ICACHE_RAM_ATTR cont_init(cont_t* cont) {
}
}

int ICACHE_RAM_ATTR cont_check(cont_t* cont) {
int cont_check(cont_t* cont) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm calling this from an ISR, so it needs to be in RAM.
My usage is as a kind of canary for stack usage. I suppose that would no longer be necessary when the breakpoint approach is implemented, but until then, I'd prefer this be kept in RAM.

if(cont->stack_guard1 != CONT_STACKGUARD || cont->stack_guard2 != CONT_STACKGUARD) return 1;

return 0;
}

int ICACHE_RAM_ATTR cont_get_free_stack(cont_t* cont) {
int cont_get_free_stack(cont_t* cont) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I intend to call this from an ISR for statistical purposes (on my todo list for my application), so I'd prefer to keep it in RAM.

@kylefleming
Copy link
Contributor Author

Updated to integrate changes request by @igrr and @devyte.

@devyte
Copy link
Collaborator

devyte commented Mar 23, 2018

@igrr I think this is ok now.

@devyte devyte merged commit 212a829 into esp8266:master Mar 31, 2018
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.

None yet

3 participants