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

Introduce constant function for further compiler optimizations #392

Open
wants to merge 1 commit into
base: dev
Choose a base branch
from

Conversation

jserv
Copy link
Contributor

@jserv jserv commented Apr 22, 2021

GNU extension __attribute__ ((const)) helps the compiler to know semantic
meaning of a function call, so that it allows higher optimization than
to normal functions. For example, Common Sub-expression Elimination
(CSE) is used to avoid duplicating the same code inside a function, and
function _mi_os_page_size can benefit from CSE optimization once it is
declared as a constant function.

Reference:
https://lwn.net/Articles/285332/

GNU extension __attribute__((const)) helps the compiler to know semantic
meaning of a function call, so that it allows higher optimization than
to normal functions. For example, Common Sub-expression Elimination
(CSE) is used to avoid duplicating the same code inside a function, and
function "_mi_os_page_size" can benefit from CSE optimization once it is
declared as a constant function.

Reference:
  https://lwn.net/Articles/285332/
@daanx
Copy link
Collaborator

daanx commented Apr 22, 2021

Thanks @jserv ! Right, I had been thinking about const and pure attributes. However, it is also tricky as we really need to make sure the annotation is correct -- if not, things can subtly go very wrong at runtime.

I think most annotations in your PR are correct, but actually the one for os_page_size is not I think -- the function has no arguments and it reads from "global" memory; indeed, the global variable is initialized at some point so it could return different values at different points (of course, we carefully (try to) ensure that we only read it after initializiation but the compiler does not know that. I guess it may work out but I think it is still incorrect -- the pure annotation is correct I guess ?

(also, we should add void to the os_page_size declaration.)

(so, overall I am hesitant with these annotations and we need to check with lots of care)

@daanx daanx changed the base branch from master to dev April 28, 2021 19:55
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