Hi,
On Wed, Jun 11, 2014 at 12:24:57PM +0400, Ilya Enkovich wrote:
> Hi,
>
> This patch fixes IPA CP pass to handle instrumented code correctly.
>
> Bootstrapped and tested on linux-x86_64.
>
> Thanks,
> Ilya
> --
> gcc/
>
> 2014-06-11 Ilya Enkovich <ilya.enkov...@intel.com>
>
> * ipa-cp.c (initialize_node_lattices): Check original
> version locality for instrumentation clones.
> (propagate_constants_accross_call): Do not propagate
> through instrumentation thunks.
>
>
> diff --git a/gcc/ipa-cp.c b/gcc/ipa-cp.c
> index 689378a..683b9f0 100644
> --- a/gcc/ipa-cp.c
> +++ b/gcc/ipa-cp.c
> @@ -699,7 +699,10 @@ initialize_node_lattices (struct cgraph_node *node)
> int i;
>
> gcc_checking_assert (cgraph_function_with_gimple_body_p (node));
> - if (!node->local.local)
> + if (!node->local.local
> + || (node->instrumentation_clone
> + && node->instrumented_version
> + && !node->instrumented_version->local.local))
This looks quite convoluted, can you please put the test into a new
predicate in cgraph.c? I assume you had to change other tests of the
local flag in a similar fashion anyway.
> {
> /* When cloning is allowed, we can assume that externally visible
> functions are not called. We will compensate this by cloning
> @@ -1440,7 +1443,8 @@ propagate_constants_accross_call (struct cgraph_edge
> *cs)
> alias_or_thunk = cs->callee;
> while (alias_or_thunk->alias)
> alias_or_thunk = cgraph_alias_target (alias_or_thunk);
> - if (alias_or_thunk->thunk.thunk_p)
> + if (alias_or_thunk->thunk.thunk_p
> + && !alias_or_thunk->thunk.add_pointer_bounds_args)
so there are thunks that do not change the first argument and so we do
want to propagate to/through it?
> {
> ret |= set_all_contains_variable (ipa_get_parm_lattices (callee_info,
> 0));
> @@ -1449,6 +1453,20 @@ propagate_constants_accross_call (struct cgraph_edge
> *cs)
> else
> i = 0;
>
> + /* No propagation through instrumentation thunks is available yet.
> + It should be possible with proper mapping of call args and
> + instrumented callee params in the propagation loop below. But
> + this case mostly occurs when legacy code calls instrumented code
> + and it is not a primary target for optimizations. */
> + if (!alias_or_thunk->instrumentation_clone
> + && callee->instrumentation_clone)
> + {
> + for (; i < parms_count; i++)
> + ret |= set_all_contains_variable (ipa_get_parm_lattices (callee_info,
> + i));
> + return ret;
> + }
> +
and these thunks are different from those marked as
thunk.add_pointer_bounds_args? If they are not, the previous hunk is
redundant.
My apologies for not looking at the patches introducing all the new
cgraph_node fields but it is quite difficult to figure out what the
new tests actually mean (that is why I'd really prefer predicates with
more explanatory names).
Thanks,
Martin
