> Hi, Insane value profile data may contain indirect call targets with
> wrong (corrupted) pids. r172276 solves the problem when the pid
> refers to a bogus target that is still 'alive'. This patch addresses
> the issue when the bogus target is already eliminated or pid is too
> large.
>
> OK after testing? (SPEC FDO testing is currently blocked by some regressions).
I will look into the FDO inliner ICEs. I added sanity check for profile updates
and it seems that insane profiles are mishandled somewhere.
> Index: cgraph.c
> ===================================================================
> --- cgraph.c (revision 172721)
> +++ cgraph.c (working copy)
> @@ -1478,6 +1478,7 @@ cgraph_remove_node (struct cgraph_node *
> cgraph_call_node_removal_hooks (node);
> cgraph_node_remove_callers (node);
> cgraph_node_remove_callees (node);
> + cgraph_remove_pid (node);
> ipa_remove_all_references (&node->ref_list);
> ipa_remove_all_refering (&node->ref_list);
> VEC_free (ipa_opt_pass, heap,
profiling is now run as IPA pass. This means that we compute pid map and
nothing can remove nodes before we read in the profile. So it this is
unnecesary to hook PID array updating into cgraph_remove_node.
> Index: value-prof.c
> ===================================================================
> --- value-prof.c (revision 172721)
> +++ value-prof.c (working copy)
> @@ -1083,13 +1083,35 @@ init_pid_map (void)
> /* Return cgraph node for function with pid */
>
> static inline struct cgraph_node*
> -find_func_by_pid (int pid)
> +find_func_by_pid (int pid)
> {
> init_pid_map ();
>
> + if (pid >= cgraph_max_pid)
> + {
> + if (flag_profile_correction)
> + inform (DECL_SOURCE_LOCATION (current_function_decl),
> + "Inconsistent profile: indirect call target (%d) does not
> exist", pid);
> + else
> + error ("Inconsistent profile: indirect call target (%d) does not
> exist", pid);
> +
> + return NULL;
> + }
> +
> return pid_map [pid];
PIDs are not dense, because functions might've been removed from before we get
to ipa-profile. I would suggest making pid_map VECtor to be more consistent
with rest of GCC sourc, making init_pid_map to allocate it cleared and add also
test that VEC_index (pid_map, pid) is not NULL in addition to the bound check
above.
Also we ought to free pids once the IPA profiling pass is done. I.e. at the end
of tree_profling
function.
Honza
