The coverage.c related patch is not uploaded properly. Will be reviewed seperately.
David https://codereview.appspot.com/7393058/diff/1/gcc/gcov-dump.c File gcc/gcov-dump.c (right): https://codereview.appspot.com/7393058/diff/1/gcc/gcov-dump.c#newcode581 gcc/gcov-dump.c:581: const char *primary_suffix = mod_info->is_primary ? "primary" : ""; Put 'auxiilary' here https://codereview.appspot.com/7393058/diff/1/gcc/gcov-dump.c#newcode583 gcc/gcov-dump.c:583: ? mod_info->is_primary ? "exported" : "auxiliary" empty string here. https://codereview.appspot.com/7393058/diff/1/libgcc/dyn-ipa.c File libgcc/dyn-ipa.c (right): https://codereview.appspot.com/7393058/diff/1/libgcc/dyn-ipa.c#newcode77 libgcc/dyn-ipa.c:77: /* Used by new algo. This dyn_pointer_set only algo --> algorithm. https://codereview.appspot.com/7393058/diff/1/libgcc/dyn-ipa.c#newcode79 libgcc/dyn-ipa.c:79: module ident. */ module ident or module idx ? https://codereview.appspot.com/7393058/diff/1/libgcc/dyn-ipa.c#newcode92 libgcc/dyn-ipa.c:92: /* used by new_alg */ new_algo --> new algorithm. https://codereview.appspot.com/7393058/diff/1/libgcc/dyn-ipa.c#newcode98 libgcc/dyn-ipa.c:98: struct modu_node { { to next line. https://codereview.appspot.com/7393058/diff/1/libgcc/dyn-ipa.c#newcode102 libgcc/dyn-ipa.c:102: unsigned char visited; /* needed? */ Remove the comment 'needed?' https://codereview.appspot.com/7393058/diff/1/libgcc/dyn-ipa.c#newcode113 libgcc/dyn-ipa.c:113: unsigned char visited; /* needed? */ Remove the field comment. https://codereview.appspot.com/7393058/diff/1/libgcc/dyn-ipa.c#newcode189 libgcc/dyn-ipa.c:189: static int flag_modu_merge_edges = 1; These two flags should have corresponding --param= https://codereview.appspot.com/7393058/diff/1/libgcc/dyn-ipa.c#newcode194 libgcc/dyn-ipa.c:194: #endif Use __gcov_lipo_max_mem which has the value specified by --param=max-lipo-mem=... https://codereview.appspot.com/7393058/diff/1/libgcc/dyn-ipa.c#newcode253 libgcc/dyn-ipa.c:253: Why is this interface needed? Can get_module_idx be used instead? Or get rid of get_module_idx, and use ident consistently (and fix up limited array reference by 1). get_module_idx_from_guid should also be changed to get_module_ident_from_guid https://codereview.appspot.com/7393058/diff/1/libgcc/dyn-ipa.c#newcode292 libgcc/dyn-ipa.c:292: Use either get_module_idx or get_module_id_value. https://codereview.appspot.com/7393058/diff/1/libgcc/dyn-ipa.c#newcode298 libgcc/dyn-ipa.c:298: pointer_set_find_or_insert (p, get_module_id_value (imp_mod)); Why not changing it to use get_module_idx? https://codereview.appspot.com/7393058/diff/1/libgcc/dyn-ipa.c#newcode333 libgcc/dyn-ipa.c:333: return get_module_id_value ((const struct gcov_info *)p); get_module_idx? https://codereview.appspot.com/7393058/diff/1/libgcc/dyn-ipa.c#newcode339 libgcc/dyn-ipa.c:339: gcc_assert (m_ix != 0); module id may be insane. Should be handled here. https://codereview.appspot.com/7393058/diff/1/libgcc/dyn-ipa.c#newcode340 libgcc/dyn-ipa.c:340: return the_dyn_call_graph.sup_modules[m_ix-1].exported_to; The input m_ix should be normalized to zero based idx already. If get_module_ident is used consistently, the parameter name should be changed to module_ident. https://codereview.appspot.com/7393058/diff/1/libgcc/dyn-ipa.c#newcode340 libgcc/dyn-ipa.c:340: return the_dyn_call_graph.sup_modules[m_ix-1].exported_to; space https://codereview.appspot.com/7393058/diff/1/libgcc/dyn-ipa.c#newcode344 libgcc/dyn-ipa.c:344: create_exported_to (unsigned m_ix) See above comment about the ident. https://codereview.appspot.com/7393058/diff/1/libgcc/dyn-ipa.c#newcode348 libgcc/dyn-ipa.c:348: gcc_assert (m_ix != 0); See above comment about assertion. https://codereview.appspot.com/7393058/diff/1/libgcc/dyn-ipa.c#newcode349 libgcc/dyn-ipa.c:349: the_dyn_call_graph.sup_modules[m_ix-1].exported_to = p space https://codereview.appspot.com/7393058/diff/1/libgcc/dyn-ipa.c#newcode355 libgcc/dyn-ipa.c:355: get_imported_modus (unsigned m_ix) See above comment about parameter name. https://codereview.appspot.com/7393058/diff/1/libgcc/dyn-ipa.c#newcode360 libgcc/dyn-ipa.c:360: gcc_assert (m_ix != 0); Assertion needs to be handled. https://codereview.appspot.com/7393058/diff/1/libgcc/dyn-ipa.c#newcode367 libgcc/dyn-ipa.c:367: = pointer_set_create (imp_mod_get_key); split this into two statements https://codereview.appspot.com/7393058/diff/1/libgcc/dyn-ipa.c#newcode456 libgcc/dyn-ipa.c:456: if (flag_alg_mode == 1) Define enum for algorithms. https://codereview.appspot.com/7393058/diff/1/libgcc/dyn-ipa.c#newcode508 libgcc/dyn-ipa.c:508: pointer_set_destroy_2 (the_dyn_call_graph.sup_modules[i].exported_to); Change the name for pointer_set_destroy_2. https://codereview.appspot.com/7393058/diff/1/libgcc/dyn-ipa.c#newcode728 libgcc/dyn-ipa.c:728: pointer_set_destroy_2 (struct dyn_pointer_set *pset) better interface name. https://codereview.appspot.com/7393058/diff/1/libgcc/dyn-ipa.c#newcode731 libgcc/dyn-ipa.c:731: XDELETEVEC (pset->slots); Unused i? https://codereview.appspot.com/7393058/diff/1/libgcc/dyn-ipa.c#newcode810 libgcc/dyn-ipa.c:810: /* Returns nonzero if PSET contains P. P must be nonnull. The comment is not matching. https://codereview.appspot.com/7393058/diff/1/libgcc/dyn-ipa.c#newcode828 libgcc/dyn-ipa.c:828: n = 0; Is it possible that there is no empty slot -- thus this cause infinite loop? https://codereview.appspot.com/7393058/diff/1/libgcc/dyn-ipa.c#newcode1254 libgcc/dyn-ipa.c:1254: static int dyn_fibheap_comp_data (dyn_fibheap_t, dyn_fibheapkey_t, void *, fibnode_t); wrap long line. https://codereview.appspot.com/7393058/diff/1/libgcc/dyn-ipa.c#newcode1281 libgcc/dyn-ipa.c:1281: dyn_fibheap_compare (dyn_fibheap_t heap ATTRIBUTE_UNUSED, fibnode_t a, fibnode_t b) wrap long line. https://codereview.appspot.com/7393058/diff/1/libgcc/dyn-ipa.c#newcode1291 libgcc/dyn-ipa.c:1291: dyn_fibheap_comp_data (dyn_fibheap_t heap, dyn_fibheapkey_t key, void *data, fibnode_t b) wrap long line. https://codereview.appspot.com/7393058/diff/1/libgcc/dyn-ipa.c#newcode1532 libgcc/dyn-ipa.c:1532: /* Compute module grouping using CUTOFF_COUNT as the hot edge One new line above. https://codereview.appspot.com/7393058/diff/1/libgcc/dyn-ipa.c#newcode1542 libgcc/dyn-ipa.c:1542: case 0: Name methods with _1, _0 properly: _1 is 'inclusion_based_with_priority'. The _0 is the old algorithm: 'eager_propagation'. https://codereview.appspot.com/7393058/diff/1/libgcc/dyn-ipa.c#newcode1550 libgcc/dyn-ipa.c:1550: { make the parameter consistent -- e.g., consistently use ident. https://codereview.appspot.com/7393058/diff/1/libgcc/dyn-ipa.c#newcode1569 libgcc/dyn-ipa.c:1569: e = XNEW (struct modu_edge); use XCNEW https://codereview.appspot.com/7393058/diff/1/libgcc/dyn-ipa.c#newcode1582 libgcc/dyn-ipa.c:1582: modu_graph_process_dyn_cgraph_node (struct dyn_cgraph_node *node, Missing comment. https://codereview.appspot.com/7393058/diff/1/libgcc/dyn-ipa.c#newcode1613 libgcc/dyn-ipa.c:1613: = XNEWVEC (struct modu_node, n_modules); Use XCNEWVEC https://codereview.appspot.com/7393058/diff/1/libgcc/dyn-ipa.c#newcode1630 libgcc/dyn-ipa.c:1630: gcc_assert (node); Avoid assertion -- add error prints -- similarly for other assertions. https://codereview.appspot.com/7393058/diff/1/libgcc/dyn-ipa.c#newcode1660 libgcc/dyn-ipa.c:1660: get_group_ggc_mem (struct dyn_pointer_set *s) Missing comments. https://codereview.appspot.com/7393058/diff/1/libgcc/dyn-ipa.c#newcode1668 libgcc/dyn-ipa.c:1668: static gcov_unsigned_t Missing comments https://codereview.appspot.com/7393058/diff/1/libgcc/dyn-ipa.c#newcode1727 libgcc/dyn-ipa.c:1727: pointer_set_traverse (s_imported_mods, modu_add_auxiliary_1, &t_mid, &count, 0); wrap long line. https://codereview.appspot.com/7393058/diff/1/libgcc/dyn-ipa.c#newcode1727 libgcc/dyn-ipa.c:1727: pointer_set_traverse (s_imported_mods, modu_add_auxiliary_1, &t_mid, &count, 0); This will cause t_mid to be added to the exported_set for all the aux modules in s_imported_modules -- is this needed? t_mid should be only added to the the root module of s_imported_mods? https://codereview.appspot.com/7393058/diff/1/libgcc/dyn-ipa.c#newcode1735 libgcc/dyn-ipa.c:1735: ps_check_ggc_mem (const void *value, Comment needed. https://codereview.appspot.com/7393058/diff/1/libgcc/dyn-ipa.c#newcode1817 libgcc/dyn-ipa.c:1817: return 0; This constraints might be too strong. For instance, if one of the exported_to module (which is not the hottest) has the memory limit exceeded -- it will block other exported_to modules from benefiting from the newly added aux module. Since the the edges are processed in priority order, it should ok to relax this to allow propagation even when exported_to module is too large. This still has the good nature of the inclusion based algo -- because the most critical part of the aux groups are still shared in the hierarchy. https://codereview.appspot.com/7393058/diff/1/libgcc/dyn-ipa.c#newcode2116 libgcc/dyn-ipa.c:2116: */ Remove this. https://codereview.appspot.com/7393058/
