Improve handling of memory operands in ipa-icf 3/4
authorJan Hubicka <jh@suse.cz>
Tue, 17 Nov 2020 14:41:06 +0000 (15:41 +0100)
committerJan Hubicka <jh@suse.cz>
Tue, 17 Nov 2020 14:41:06 +0000 (15:41 +0100)
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.

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.

gcc/ipa-icf.c
gcc/ipa-icf.h

index a283195a487431b49c5c96a44effabd1fbf5009a..27eeda3a319e30dc150a7dcddbe10a107c40b366 100644 (file)
@@ -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,30 @@ 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));
+               tree t = ao_ref_alias_ptr_type (&ref);
+               if (variably_modified_type_p (t, NULL_TREE))
+                 memory_access_types.safe_push (t);
+               t = ao_ref_base_alias_ptr_type (&ref);
+               if (variably_modified_type_p (t, NULL_TREE))
+                 memory_access_types.safe_push (t);
+             }
+         }
        /* Consider nocf_check attribute in hash as it affects code
           generation.  */
        if (code == GIMPLE_CALL
@@ -2129,6 +2154,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 +2213,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 +2421,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 +2559,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
index 6dc1a135444d69714cdde30573fa345c02e1623d..3e0e72428f73d1206b49c3b8ac8df83c8b86d4f5 100644 (file)
@@ -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);