https://gcc.gnu.org/bugzilla/show_bug.cgi?id=61776
davidxl <xinliangli at gmail dot com> changed:
What |Removed |Added
----------------------------------------------------------------------------
CC| |xinliangli at gmail dot com
--- Comment #3 from davidxl <xinliangli at gmail dot com> ---
In tree-profiling (), in the intrumentation loop (for each def function) -- a
pending list of functions calling pure functions should be recorded.
After the loop that reset the const/pure flags, a loop should iterate though
the pending list, and perform cfg fixup on them.
David
(In reply to Richard Biener from comment #1)
> There are several possibilities:
>
> - don't instrument pure/const functions
> - do not reset the pure/const flag (I see no reason to - the compiler might
> not insert side-effects and we can consider the counter updates as
> non-side-effect)
>
> That is, I wonder why we do
>
> /* Drop pure/const flags from instrumented functions. */
> FOR_EACH_DEFINED_FUNCTION (node)
> {
> if (!gimple_has_body_p (node->decl)
> || !(!node->clone_of
> || node->decl != node->clone_of->decl))
> continue;
>
> /* Don't profile functions produced for builtin stuff. */
> if (DECL_SOURCE_LOCATION (node->decl) == BUILTINS_LOCATION)
> continue;
>
> cgraph_set_const_flag (node, false, false);
> cgraph_set_pure_flag (node, false, false);
> }
>
> but don't then call execute_fixup_cfg () again (but it doesn't yet deal
> with a const/pure function becoming non-pure/const and thus a bb-ending
> stmt).
>
> Btw, this is yet another case where transitioning to call flags instead
> of decl flags would save us - definitely even the instrumented call cannot
> return abnormally.
>
> Dropping the clearing of const/pure fixes the bug. Honza? I only can think
> of the case where we have
>
> for (..)
> {
> const ();
> const ();
> }
>
> and inline only one of the calls where after that loop store-motion might
> apply store-motion to the counter updates of the inline clone, overwriting
> the updates from the non-inline call. But do we really care?
>
> Anyway, confirmed. Btw, goo() should be leaf but it seems we don't
> auto-compute 'leaf' yet?
