This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: FDO usability: pid handling
Here is the revised patch. Basic FDO testing went fine. I still saw
the ipa-inline assertion in SPEC FDO. Will run it when the regression
is fixed.
Thanks,
David
On Tue, Apr 19, 2011 at 5:33 PM, Jan Hubicka <hubicka@ucw.cz> wrote:
>> On Tue, Apr 19, 2011 at 4:49 PM, Jan Hubicka <hubicka@ucw.cz> wrote:
>> >> Actually, among all the choices, funcdef_no is probably the most dense
>> >> one -- it is for function decl with definition only. ?In LIPO, the
>> >
>> > Yes, funddef_no is densiest, but we don't really need great density here
>> > (in many other places we index arrays by cgraph_uid - it is intended for
>> > that purpose and we pay some attention to recycle unused nodes).
>> >
>>
>> That does not mean it is right to use sparse ids:) ?DECL_UID will be
>> the worst amongst them.
>
> Sure, that is why I suggested cgraph->uid. ?That one is kept dense and it also
> tracks cgraph node creation order. Unlike pid it counts also functions w/o
> bodies.
>> > We only care to avoid divergence in the indexes in between instrumentation
>> > and feedback compilation. ?With the IPA pass organizatoin the compiler doesn't
>> > really diverge until the profile pass, so it seems to me that all should be safe.
>>
>> When I said 'fragile' -- I meant it depends on the optimization pass
>> phase ordering which can change in the future.
>
> Well, that is the case of all of them (passes can create function bodies that
> can make funcdef_no also diverge). ?This is the case of couple passes already,
> especially OMP lowering and friends.
>
>> Ok, I will throw away pid and use funcdef_no for now. ?For future
>> replacement for the function ids, please consider the following
>> desired properties:
>>
>> 1) The id sequence does not depend on optimization passes -- only
>> depend on source/parsing order;
>
> It depends on optimization, too. ?This is why we actually have cgraph->order
> that is used for -fno-toplevel-reorder and is similar to funcdef_no, but
> assigned at finalization time.
>
>> 2) It is dense and sequential for defined functions
>> ? ?a) it has proven to be very useful to use nice looking, sequential
>> ids in triaging coverage mismatch related problems (the tree dump
>> should also show the function id);
>
> You get the cgraph uids in the dumps already.
>> ? ?b) it can be very useful in bug triaging via binary search by
>> specifying ranges of function ids (to enable optimizations etc).
>
> But as you wish, we can process with fundef_no first and then discuss
> removal of that field later.
>
> Honza
>>
>> Thanks,
>>
>> David
>>
>>
>> >
>> > Honza
>> >>
>> >> David
>> >>
>> >>
>> >>
>> >> >
>> >> > Honza
>> >> >
>> >
>
Index: cgraph.c
===================================================================
--- cgraph.c (revision 172781)
+++ cgraph.c (working copy)
@@ -142,9 +142,6 @@ int cgraph_max_uid;
/* Maximal uid used in cgraph edges. */
int cgraph_edge_max_uid;
-/* Maximal pid used for profiling */
-int cgraph_max_pid;
-
/* Set when whole unit has been analyzed so we can access global info. */
bool cgraph_global_info_ready = false;
@@ -472,7 +469,6 @@ cgraph_create_node_1 (void)
struct cgraph_node *node = cgraph_allocate_node ();
node->next = cgraph_nodes;
- node->pid = -1;
node->order = cgraph_order++;
if (cgraph_nodes)
cgraph_nodes->previous = node;
@@ -1827,8 +1823,7 @@ dump_cgraph_node (FILE *f, struct cgraph
struct cgraph_edge *edge;
int indirect_calls_count = 0;
- fprintf (f, "%s/%i(%i)", cgraph_node_name (node), node->uid,
- node->pid);
+ fprintf (f, "%s/%i", cgraph_node_name (node), node->uid);
dump_addr (f, " @", (void *)node);
if (DECL_ASSEMBLER_NAME_SET_P (node->decl))
fprintf (f, " (asm: %s)", IDENTIFIER_POINTER (DECL_ASSEMBLER_NAME (node->decl)));
Index: cgraph.h
===================================================================
--- cgraph.h (revision 172781)
+++ cgraph.h (working copy)
@@ -200,9 +200,6 @@ struct GTY((chain_next ("%h.next"), chai
/* Ordering of all cgraph nodes. */
int order;
- /* unique id for profiling. pid is not suitable because of different
- number of cfg nodes with -fprofile-generate and -fprofile-use */
- int pid;
enum ld_plugin_symbol_resolution resolution;
/* Set when function must be output for some reason. The primary
@@ -472,7 +469,6 @@ extern GTY(()) struct cgraph_node *cgrap
extern GTY(()) int cgraph_n_nodes;
extern GTY(()) int cgraph_max_uid;
extern GTY(()) int cgraph_edge_max_uid;
-extern GTY(()) int cgraph_max_pid;
extern bool cgraph_global_info_ready;
enum cgraph_state
{
@@ -730,6 +726,8 @@ void cgraph_clone_inlined_nodes (struct
void compute_inline_parameters (struct cgraph_node *);
cgraph_inline_failed_t cgraph_edge_inlinable_p (struct cgraph_edge *);
+void cgraph_init_node_map (void);
+void cgraph_del_node_map (void);
/* Create a new static variable of type TYPE. */
tree add_new_static_var (tree type);
Index: value-prof.c
===================================================================
--- value-prof.c (revision 172781)
+++ value-prof.c (working copy)
@@ -1059,35 +1059,63 @@ gimple_mod_subtract_transform (gimple_st
return true;
}
-static struct cgraph_node** pid_map = NULL;
+typedef struct
+{
+ struct cgraph_node *n;
+} cgraph_node_ptr_t;
-/* Initialize map of pids (pid -> cgraph node) */
+DEF_VEC_O (cgraph_node_ptr_t);
+DEF_VEC_ALLOC_O (cgraph_node_ptr_t, heap);
-static void
-init_pid_map (void)
+static VEC(cgraph_node_ptr_t, heap) *cgraph_node_map = NULL;
+
+/* Initialize map from FUNCDEF_NO to CGRAPH_NODE. */
+
+void
+cgraph_init_node_map (void)
{
struct cgraph_node *n;
- if (pid_map != NULL)
- return;
-
- pid_map = XCNEWVEC (struct cgraph_node*, cgraph_max_pid);
+ VEC_safe_grow_cleared (cgraph_node_ptr_t, heap,
+ cgraph_node_map, get_last_funcdef_no ());
for (n = cgraph_nodes; n; n = n->next)
{
- if (n->pid != -1)
- pid_map [n->pid] = n;
+ if (DECL_STRUCT_FUNCTION (n->decl))
+ VEC_index (cgraph_node_ptr_t, cgraph_node_map,
+ DECL_STRUCT_FUNCTION (n->decl)->funcdef_no)->n = n;
}
}
+/* Delete the CGRAPH_NODE_MAP. */
+
+void
+cgraph_del_node_map (void)
+{
+ VEC_free (cgraph_node_ptr_t, heap, cgraph_node_map);
+ cgraph_node_map = NULL;
+}
+
/* Return cgraph node for function with pid */
static inline struct cgraph_node*
-find_func_by_pid (int pid)
+find_func_by_funcdef_no (int func_id)
{
- init_pid_map ();
+ int max_id = get_last_funcdef_no ();
+ if (func_id >= max_id || VEC_index (cgraph_node_ptr_t,
+ cgraph_node_map,
+ func_id)->n == NULL)
+ {
+ if (flag_profile_correction)
+ inform (DECL_SOURCE_LOCATION (current_function_decl),
+ "Inconsistent profile: indirect call target (%d) does not exist", func_id);
+ else
+ error ("Inconsistent profile: indirect call target (%d) does not exist", func_id);
+
+ return NULL;
+ }
- return pid_map [pid];
+ return VEC_index (cgraph_node_ptr_t, cgraph_node_map, func_id)->n;
}
/* Perform sanity check on the indirect call target. Due to race conditions,
@@ -1285,7 +1313,7 @@ gimple_ic_transform (gimple stmt)
prob = (count * REG_BR_PROB_BASE + all / 2) / all;
else
prob = 0;
- direct_call = find_func_by_pid ((int)val);
+ direct_call = find_func_by_funcdef_no ((int)val);
if (direct_call == NULL)
return false;
Index: cgraphunit.c
===================================================================
--- cgraphunit.c (revision 172781)
+++ cgraphunit.c (working copy)
@@ -348,7 +348,6 @@ cgraph_finalize_function (tree decl, boo
if (node->local.finalized)
cgraph_reset_node (node);
- node->pid = cgraph_max_pid ++;
notice_global_symbol (decl);
node->local.finalized = true;
node->lowered = DECL_STRUCT_FUNCTION (decl)->cfg != NULL;
Index: function.c
===================================================================
--- function.c (revision 172781)
+++ function.c (working copy)
@@ -4379,6 +4379,13 @@ get_next_funcdef_no (void)
return funcdef_no++;
}
+/* Return value of funcdef. */
+int
+get_last_funcdef_no (void)
+{
+ return funcdef_no;
+}
+
/* Allocate a function structure for FNDECL and set its contents
to the defaults. Set cfun to the newly-allocated object.
Some of the helper functions invoked during initialization assume
Index: function.h
===================================================================
--- function.h (revision 172781)
+++ function.h (working copy)
@@ -755,6 +755,7 @@ extern bool reference_callee_copied (CUM
extern void used_types_insert (tree);
extern int get_next_funcdef_no (void);
+extern int get_last_funcdef_no (void);
/* In predict.c */
extern bool optimize_function_for_size_p (struct function *);
Index: tree-profile.c
===================================================================
--- tree-profile.c (revision 172781)
+++ tree-profile.c (working copy)
@@ -369,7 +369,7 @@ gimple_gen_ic_func_profiler (void)
ptr_var = force_gimple_operand_gsi (&gsi, ic_void_ptr_var,
true, NULL_TREE, true,
GSI_SAME_STMT);
- tree_uid = build_int_cst (gcov_type_node, c_node->pid);
+ tree_uid = build_int_cst (gcov_type_node, current_function_funcdef_no);
stmt1 = gimple_build_call (tree_indirect_call_profiler_fn, 4,
counter_ptr, tree_uid, cur_func, ptr_var);
gsi_insert_before (&gsi, stmt1, GSI_SAME_STMT);
@@ -454,6 +454,8 @@ tree_profiling (void)
if (cgraph_state == CGRAPH_STATE_FINISHED)
return 0;
+ cgraph_init_node_map();
+
for (node = cgraph_nodes; node; node = node->next)
{
if (!node->analyzed
@@ -548,6 +550,7 @@ tree_profiling (void)
pop_cfun ();
}
+ cgraph_del_node_map();
return 0;
}