#1975: Cactus: mark routines used by DECLARE_CCTK_ARGUMENTS as pure -------------------------+-------------------------------------------------- Reporter: rhaas | Owner: Type: enhancement | Status: new Priority: minor | Milestone: Component: Cactus | Version: development version Keywords: | -------------------------+-------------------------------------------------- Currently CCTK_DECLARE_ARGUMENTS contains code like this:
{{{ static int varindex = -100; if (varindex == -100) varindex = CCTK_VarIndex("thorn::var"); var = CCTKi_VarDataPtr(cctGH, 0, varindex); }}}
ie it calls the two functions VarIndex and VarDataPtr for all variables that the thorn defines or inherits. This happens no matter whether the variable "var" is later used in the routine or not since the two functions could have side effects. The patch in the pull requests declares the functions "pure", ie side effect free (https://gcc.gnu.org/onlinedocs/gcc /Common-Function-Attributes.html#index-functions-that-have-no-side- effects-3320) which lets the compiler remove the call to the functions if their result is not used.
Technically CCTK_VarIndex does have a side effect: it can call CCTK_VWarn (at level 6) for unknown variable names though the result does not affect the computation. CCTKi_VarDataPtr is explicitly designed to be side-effect free.
The patch would lead to slightly smaller code and possible slightly faster code as well when CCTK_DECLARE_ARGUMENTS is used often or with many inherited variables.
The pull request is at https://bitbucket.org/cactuscode/cactus/pull- requests/34/cactus-mark-routines-used-by/diff
#1975: Cactus: mark routines used by DECLARE_CCTK_ARGUMENTS as pure --------------------------+------------------------------------------------- Reporter: rhaas | Owner: Type: enhancement | Status: new Priority: minor | Milestone: Component: Cactus | Version: development version Resolution: | Keywords: --------------------------+-------------------------------------------------
Comment (by anonymous):
Technically, CCTK_VarIndex and CCTKi_VarDataPtr are not pure, since they depend on global state in addition to their arguments. I cannot think of an optimization that GCC or Clang would perform where this difference would matter, but we should be careful nonetheless.
For example, the following (theoretical) optimizations would cause problems: - GCC calls CCTK_VarIndex during initialization (i.e. when the program starts up) instead of during run time; at that time, the parameter file has not been read, thorns have not bee activated, and variable indices are not yet known - GCC memoizes the variable pointer, which breaks when AMR is used and the variable pointer differs over time - GCC aggressively inlines (e.g. for a small test case), so that a thorn's routine is inlined into Carpet's schedule loop, making the different variable pointers visible without memoization
I don't see any of these optimizations happening at the moment. Nevertheless, we should be aware that we're breaking GCC's assumptions here.
In the mean time I'm all for it; reducing this overhead is important.
A point of reference -- I tried declaring several Carpet routines "pure" or "const" in the past, and this led to random segfaults. I had to undo these changes. However, I suspect that at that time, the problem was code generation errors in the compiler. I hope that compilers have improved in the mean time.
#1975: Cactus: mark routines used by DECLARE_CCTK_ARGUMENTS as pure --------------------------+------------------------------------------------- Reporter: rhaas | Owner: Type: enhancement | Status: review Priority: minor | Milestone: Component: Cactus | Version: development version Resolution: | Keywords: --------------------------+------------------------------------------------- Changes (by rhaas):
* status: new => review
Comment:
As far as I know, dependence on global variables is allowed for pure functions (from the linked gcc docs): {{{ pure Many functions have no effects except the return value and their return value depends only on the parameters and/or global variables. }}} and eg strlen is given as an example.
They are not "const" since their result depends on more than just the *values* of their arguments (they inspect the content of the pointers and the gcc state that for const "Basically this is just slightly more strict class than the pure attribute below, since function is not allowed to read global memory.").
I think one has to be very careful with what is const and what is pure. Code generation errors may be a different matter.
#1975: Cactus: mark routines used by DECLARE_CCTK_ARGUMENTS as pure --------------------------+------------------------------------------------- Reporter: rhaas | Owner: Type: enhancement | Status: reviewed_ok Priority: minor | Milestone: Component: Cactus | Version: development version Resolution: | Keywords: --------------------------+------------------------------------------------- Changes (by eschnett):
* status: review => reviewed_ok
Comment:
Approved, but only after the release.
#1975: Cactus: mark routines used by DECLARE_CCTK_ARGUMENTS as pure --------------------------+------------------------------------------------- Reporter: rhaas | Owner: Type: enhancement | Status: closed Priority: minor | Milestone: Component: Cactus | Version: development version Resolution: fixed | Keywords: --------------------------+------------------------------------------------- Changes (by rhaas):
* status: reviewed_ok => closed * resolution: => fixed
Comment:
Applied as git hash c6cd8329 of the flesh.
trac@lists.einsteintoolkit.org
-
Einstein Toolkit