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

Faster ISR sharing in ASM #243

Merged
merged 1 commit into from
Dec 11, 2019
Merged

Faster ISR sharing in ASM #243

merged 1 commit into from
Dec 11, 2019

Conversation

ceres-c
Copy link
Contributor

@ceres-c ceres-c commented Dec 8, 2019

While in the process of writing a new codec I stumbled upon the long
standing issue of ISR sharing in AVR MCUs.
Actually this is accomplished with an ISR written in C, which simply
calls another plain C function referenced via a function pointer
updated at runtime.
This approach is slow because GCC can't optimize the ISR, since it
does not know which registers will be used, so it defaults to push/pop
all of them.

My solution keeps the concept of pointers to functions, but greatly
improves speed by reducing the ISR itself to the bare minimum to call
another function, which will be compiled by GCC as a signal.
This means all interrupt optimizations will be put in place by GCC,
while ISRs can be still written in plain C code, but the overhead is
now much smaller. It has proven to reduce by 20 the number of
instructions for every ISR, mainly pushes/pops, which cuts the clock
cycle count down by 30 cycles (1 cycle for every push, 2 for pop).
In time units, this means 1.1 uSec are saved for every ISR invocation
and every shared ISR now takes only 13 clock cycles more than the
"bare" one.

A new ISR_SHARED function type has been defined in Common.h to hide
away from the programmer GCC attributes. All new shared interrupt
handling routines should be defined of this type to prevent
stack and registers corruption.

Minor changes were made to Codec.h to allow including it in .S files.

While in the process of writing a new codec I stumbled upon the long
standing issue of ISR sharing in AVR MCUs.
Actually this is accomplished with an ISR written in C, which simply
calls another plain C function referenced via a function pointer
updated at runtime.
This approach is slow because GCC can't optimize the ISR, since it
does not know which registers will be used, so it defaults to push/pop
all of them.

My solution keeps the concept of pointers to functions, but greatly
improves speed by reducing the ISR itself to the bare minimum to call
another function, which will be compiled by GCC as a signal.
This means all interrupt optimizations will be put in place by GCC,
while ISRs can be still written in plain C code, but the overhead is
now much smaller. It has proven to reduce by 20 the number of
instructions for every ISR, mainly pushes/pops, which cuts the clock
cycle count down by 30 cycles (1 cycle for every push, 2 for pop).
In time units, this means 1.1 uSec are saved for every ISR invocation
and every shared ISR now takes only 13 clock cycles more than the
"bare" one.

A new ISR_SHARED function type has been defined in Common.h to hide
away from the programmer GCC attributes. All new shared interrupt
handling routines should be defined of this type to prevent
stack and registers corruption.

Minor changes were made to Codec.h to allow including it in .S files.
@MrMoDDoM
Copy link
Contributor

MrMoDDoM commented Dec 9, 2019

Furthermore, this solution prevents all interrupt signal handler from being occupied, thus allowing other developers to use them dynamically for their code.

In particular, when a codec uses a port signal, this signal can no longer be reused by other codecs unless the first one is excluded from being compiled.

This pull request address this problem and should not harm any of the codecs or signal handler actually present in the code base.

@fptrs fptrs merged commit 868d8b6 into emsec:master Dec 11, 2019
@fptrs
Copy link
Collaborator

fptrs commented Dec 11, 2019

Hi @ceres-c,
great work so far. You might also want to take a look at assembly macros in order to substitute the code to call isr_funcs within the ISRSharing.S.

@ceres-c
Copy link
Contributor Author

ceres-c commented Dec 11, 2019

You're right, I didn't think about it in the first place.
For now I'd leave the code as it is; it could be changed later if more copy-paste will be needed, I guess.

@fptrs
Copy link
Collaborator

fptrs commented Dec 13, 2019

I just added it so done is done 👍

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.

3 participants