Improve handling of memory operands in ipa-icf 3/4
Jan Hubicka
hubicka@ucw.cz
Fri Nov 13 17:50:26 GMT 2020
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 <hubicka@ucw.cz>
Martin Liska <mliska@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);
More information about the Gcc-patches
mailing list