Hi, this patch is based on Maritn's patch https://gcc.gnu.org/legacy-ml/gcc-patches/2019-11/msg02633.html however based on new code that track and compare memory accesses so it can be implemented correctly.
As shown here https://gcc.gnu.org/pipermail/gcc-patches/2020-November/558773.html the most common reason for function body being streamed in but merging to fail is the mismatch in base alias set. This patch collect base and ref types ao_alias_ptr types, stream them to WPA and at WPA time hash is produced. Now we can use alias_sets since these these are assumed to be same as ltrans time alias sets. This is currently not always true - but that is pre-existing issue. I will try to produce a testcase and make followup patch on this (that will stream out ODR types with TYPE_CANONICAL that is !ODR as !ODR type). However for this patch this is not a problem since the real alias sets are finer but definitly not coarser. We may make it possible to use canonical type hash and save some streaming, but I think it would be better to wait for next stage1 since it is not completely trivial WRT ODR types: either we hash ODR type names and then hash values would be too coarse for cases we got conflict betwen C and C++ type or we do not stream and will again get into trouble with hash values being too weak. Tried that - we get a lot of types that are struturally same but distinguished by ODR names (from template instantiations). As followup I will add code for merging with mismatched base alias sets. This makes the aforementioned problem about ODR names less pronounced but it is still present on pointer loads/stores which requires REF alias set mismatches. Building cc1plus memory usage goes from: Time variable usr sys wall GGC ipa lto gimple in : 2.67 ( 2%) 2.08 ( 21%) 4.63 ( 3%) 231M ( 12%) ipa lto decl in : 6.45 ( 5%) 0.53 ( 5%) 7.19 ( 5%) 461M ( 25%) ipa icf : 13.65 ( 10%) 3.73 ( 37%) 17.41 ( 12%) 164M ( 9%) TOTAL : 137.91 9.97 148.48 1868M Time variable usr sys wall GGC ipa lto gimple in : 1.59 ( 1%) 0.95 ( 19%) 2.82 ( 2%) 137M ( 9%) ipa lto decl in : 6.58 ( 5%) 0.51 ( 10%) 7.30 ( 6%) 459M ( 29%) ipa icf : 4.32 ( 4%) 0.24 ( 5%) 4.69 ( 4%) 15M ( 1%) TOTAL : 122.76 5.12 128.49 1604M Time is not 100% reliable, machine was not quiet, but we have 16% memory allocation improvement and almost 50% gimple streaming in. 6200 functions are identified in both builds. Stats listed by Martin changes from: Init called for 14675 items (23.61%). Totally needed symbols: 8008, fraction of loaded symbols: 54.57% to: Init called for 8753 items (14.08%). Totally needed symbols: 8008, fraction of loaded symbols: 91.49% Memory use will again get worse with base alias set merging, but at east it will pay back by actual code size reductions. The resons for mismatch now looks as folows: 1 false returned: 'case high values are different' in compare_gimple_switch at ../../gcc/ipa-icf-gimple.c:789 1 false returned: 'Declaration mismatch' in equals at ../../gcc/ipa-icf.c:1799 1 false returned: 'DECL_CXX_DESTRUCTOR mismatch' in equals_wpa at ../../gcc/ipa-icf.c:565 1 false returned: '' in compare_gimple_call at ../../gcc/ipa-icf-gimple.c:607 2 fprintf (_1, " false returned: \'\' in %s at %s:%u\n", func_4(D), filename_5(D), line_6(D)); 2 fprintf (_1, " false returned: \'%s\' in %s at %s:%u\n", message_6(D), func_7(D), filename_8(D), line_9(D)); 4 false returned: 'final flag mismatch' in compare_referenced_symbol_properties at ../../gcc/ipa-icf.c:401 5 false returned: 'different decl attributes' in equals_wpa at ../../gcc/ipa-icf.c:662 7 false returned: 'compare_ao_refs failed (access path difference)' in compare_operand at ../../gcc/ipa-icf-gimple.c:346 10 false returned: 'case low values are different' in compare_gimple_switch at ../../gcc/ipa-icf-gimple.c:783 10 false returned: 'different references' in compare_symbol_references at ../../gcc/ipa-icf.c:465 11 false returned: 'size mismatch' in equals_wpa at ../../gcc/ipa-icf.c:1648 18 false returned: 'variables types are different' in equals at ../../gcc/ipa-icf.c:1694 20 false returned: 'METHOD_TYPE and FUNCTION_TYPE mismatch' in equals_wpa at ../../gcc/ipa-icf.c:673 27 false returned: 'GIMPLE LHS type mismatch' in compare_gimple_assign at ../../gcc/ipa-icf-gimple.c:695 35 false returned: 'INTEGER_CST precision mismatch' in equals at ../../gcc/ipa-icf.c:1803 40 false returned: 'GIMPLE call operands are different' in compare_gimple_call at ../../gcc/ipa-icf-gimple.c:656 49 false returned: '' in operand_equal_p at ../../gcc/ipa-icf-gimple.c:282 50 false returned: 'compare_ao_refs failed (semantic difference)' in compare_operand at ../../gcc/ipa-icf-gimple.c:337 72 false returned: 'types are not compatible' in compatible_types_p at ../../gcc/ipa-icf-gimple.c:212 108 false returned: 'parameter type is not compatible' in compatible_parm_types_p at ../../gcc/ipa-icf.c:512 119 false returned: '' in compare_decl at ../../gcc/ipa-icf-gimple.c:162 156 false returned: 'inline attributes are different' in compare_referenced_symbol_properties at ../../gcc/ipa-icf.c:350 488 false returned: 'parameter types are not compatible' in equals_wpa at ../../gcc/ipa-icf.c:639 630 false returned: '' in compare_phi_node at ../../gcc/ipa-icf.c:1577 630 false returned: 'PHI node comparison returns false' in equals_private at ../../gcc/ipa-icf.c:921 1007 false returned: 'result types are different' in equals_wpa at ../../gcc/ipa-icf.c:621 1043 false returned: 'decl_or_type flags are different' in equals_wpa at ../../gcc/ipa-icf.c:572 1088 false returned: 'different tree types' in compatible_types_p at ../../gcc/ipa-icf-gimple.c:206 2819 false returned: 'GIMPLE assignment operands are different' in compare_gimple_assign at ../../gcc/ipa-icf-gimple.c:700 3000 false returned: '' in equals_private at ../../gcc/ipa-icf.c:886 3552 false returned: 'operand_equal_p failed' in compare_operand at ../../gcc/ipa-icf-gimple.c:357 This can be compared with: https://gcc.gnu.org/pipermail/gcc-patches/2020-November/558773.html Bootstrapped/regtested x86_64-linux. I plan to commit it on monday if there are no complains. Honza gcc/ChangeLog: 2020-11-13 Jan Hubicka <hubi...@ucw.cz> Martin Liska <mli...@suse.cz> * ipa-icf.c: Include data-streamer.h and alias.h. (sem_function::sem_function): Initialize memory_access_types and m_alias_sets_hash. (sem_function::hash_stmt): For memory accesses and when going to do lto streaming add base and ref types into memory_access_types. (sem_item_optimizer::write_summary): Stream memory access types. (sem_item_optimizer::read_section): Likewise and also iniitalize m_alias_sets_hash. (sem_item_optimizer::execute): Call sem_item_optimizer::update_hash_by_memory_access_type. (sem_item_optimizer::update_hash_by_memory_access_type): Updat. * ipa-icf.h (sem_function): Add memory_access_types and m_alias_sets_hash. diff --git a/gcc/ipa-icf.c b/gcc/ipa-icf.c index a283195a487..a0f181b5763 100644 --- a/gcc/ipa-icf.c +++ b/gcc/ipa-icf.c @@ -66,6 +66,7 @@ along with GCC; see the file COPYING3. If not see #include "coverage.h" #include "gimple-pretty-print.h" #include "data-streamer.h" +#include "tree-streamer.h" #include "fold-const.h" #include "calls.h" #include "varasm.h" @@ -86,6 +87,7 @@ along with GCC; see the file COPYING3. If not see #include "dbgcnt.h" #include "tree-vector-builder.h" #include "symtab-thunks.h" +#include "alias.h" using namespace ipa_icf_gimple; @@ -227,14 +229,16 @@ hash_map<const_tree, hashval_t> sem_item::m_type_hash_cache; /* Semantic function constructor that uses STACK as bitmap memory stack. */ sem_function::sem_function (bitmap_obstack *stack) -: sem_item (FUNC, stack), m_checker (NULL), m_compared_func (NULL) + : sem_item (FUNC, stack), memory_access_types (), m_alias_sets_hash (0), + m_checker (NULL), m_compared_func (NULL) { bb_sizes.create (0); bb_sorted.create (0); } sem_function::sem_function (cgraph_node *node, bitmap_obstack *stack) -: sem_item (FUNC, node, stack), m_checker (NULL), m_compared_func (NULL) + : sem_item (FUNC, node, stack), memory_access_types (), + m_alias_sets_hash (0), m_checker (NULL), m_compared_func (NULL) { bb_sizes.create (0); bb_sorted.create (0); @@ -1438,9 +1442,27 @@ sem_function::hash_stmt (gimple *stmt, inchash::hash &hstate) /* All these statements are equivalent if their operands are. */ for (unsigned i = 0; i < gimple_num_ops (stmt); ++i) - m_checker->hash_operand (gimple_op (stmt, i), hstate, 0, - func_checker::get_operand_access_type - (&map, gimple_op (stmt, i))); + { + func_checker::operand_access_type + access_type = func_checker::get_operand_access_type + (&map, gimple_op (stmt, i)); + m_checker->hash_operand (gimple_op (stmt, i), hstate, 0, + access_type); + /* For memory accesses when hasing for LTO stremaing record + base and ref alias ptr types so we can compare them at WPA + time without having to read actual function body. */ + if (access_type == func_checker::OP_MEMORY + && lto_streaming_expected_p () + && flag_strict_aliasing) + { + ao_ref ref; + + ao_ref_init (&ref, gimple_op (stmt, i)); + memory_access_types.safe_push (ao_ref_alias_ptr_type (&ref)); + memory_access_types.safe_push + (ao_ref_base_alias_ptr_type (&ref)); + } + } /* Consider nocf_check attribute in hash as it affects code generation. */ if (code == GIMPLE_CALL @@ -2129,6 +2151,14 @@ sem_item_optimizer::write_summary (void) streamer_write_uhwi_stream (ob->main_stream, node_ref); streamer_write_uhwi (ob, (*item)->get_hash ()); + + if ((*item)->type == FUNC) + { + sem_function *fn = static_cast<sem_function *> (*item); + streamer_write_uhwi (ob, fn->memory_access_types.length ()); + for (unsigned i = 0; i < fn->memory_access_types.length (); i++) + stream_write_tree (ob, fn->memory_access_types[i], true); + } } } @@ -2180,6 +2210,18 @@ sem_item_optimizer::read_section (lto_file_decl_data *file_data, cgraph_node *cnode = dyn_cast <cgraph_node *> (node); sem_function *fn = new sem_function (cnode, &m_bmstack); + unsigned count = streamer_read_uhwi (&ib_main); + inchash::hash hstate (0); + if (flag_incremental_link == INCREMENTAL_LINK_LTO) + fn->memory_access_types.reserve_exact (count); + for (unsigned i = 0; i < count; i++) + { + tree type = stream_read_tree (&ib_main, data_in); + hstate.add_int (get_deref_alias_set (type)); + if (flag_incremental_link == INCREMENTAL_LINK_LTO) + fn->memory_access_types.quick_push (type); + } + fn->m_alias_sets_hash = hstate.end (); fn->set_hash (hash); m_items.safe_push (fn); } @@ -2376,6 +2418,7 @@ sem_item_optimizer::execute (void) build_graph (); update_hash_by_addr_refs (); + update_hash_by_memory_access_type (); build_hash_based_classes (); if (dump_file) @@ -2513,6 +2556,21 @@ sem_item_optimizer::update_hash_by_addr_refs () m_items[i]->set_hash (m_items[i]->global_hash); } +void +sem_item_optimizer::update_hash_by_memory_access_type () +{ + for (unsigned i = 0; i < m_items.length (); i++) + { + if (m_items[i]->type == FUNC) + { + sem_function *fn = static_cast<sem_function *> (m_items[i]); + inchash::hash hstate (fn->get_hash ()); + hstate.add_int (fn->m_alias_sets_hash); + fn->set_hash (hstate.end ()); + } + } +} + /* Congruence classes are built by hash value. */ void diff --git a/gcc/ipa-icf.h b/gcc/ipa-icf.h index 6dc1a135444..3e0e72428f7 100644 --- a/gcc/ipa-icf.h +++ b/gcc/ipa-icf.h @@ -371,12 +371,19 @@ public: /* GIMPLE codes hash value. */ hashval_t gcode_hash; + /* Vector of subpart of memory access types. */ + vec<tree> memory_access_types; + /* Total number of SSA names used in the function. */ unsigned ssa_names_size; /* Array of structures for all basic blocks. */ vec <ipa_icf_gimple::sem_bb *> bb_sorted; + /* Hash of canonical types used for memory references in the + function. */ + hashval_t m_alias_sets_hash; + /* Return true if parameter I may be used. */ bool param_used_p (unsigned int i); @@ -541,6 +548,9 @@ private: /* For each semantic item, append hash values of references. */ void update_hash_by_addr_refs (); + /* Update hash by canonical types of memory accesses. */ + void update_hash_by_memory_access_type (); + /* Congruence classes are built by hash value. */ void build_hash_based_classes (void);
