This fixes the alias check in terminate_all_aliasing_chains -- the
base we use for the hash table indexing does not constitute an object
that aliases all stores in the chain (consider for example the MEM_REF
handling which strips the offset completely).

I've refactored data structures a bit in the process of making
(as a followup) 'base_addr' a true address (and thus avoid building
that new MEM_REF for example).  A followup will then try to support
(some) bases with offset != NULL_TREE.

Bootstrap / regtest running on x86_64-unknown-linux-gnu.

Richard.

2016-11-02  Richard Biener  <rguent...@suse.de>

        * gimple-ssa-store-merging.c (struct store_immediate_info): Remove
        redundant val and dest members.
        (store_immediate_info::store_immediate_info): Adjust.
        (merged_store_group::merged_store_group): Adjust.
        (merged_store_group::apply_stores): Likewise.
        (struct imm_store_chain_info): Add base_addr field.
        (imm_store_chain_info::imm_store_chain_info): New constructor.
        (imm_store_chain_info::terminate_and_process_chain): Do not pass base.
        (imm_store_chain_info::output_merged_store): Likewise.
        (imm_store_chain_info::output_merged_stores): Likewise.
        (pass_tree_store_merging::terminate_all_aliasing_chains): Take
        imm_store_chain_info instead of base.  Fix alias check.
        (pass_tree_store_merging::terminate_and_release_chain): Likewise.
        (imm_store_chain_info::coalesce_immediate_stores): Adjust.

Index: gcc/gimple-ssa-store-merging.c
===================================================================
--- gcc/gimple-ssa-store-merging.c      (revision 241779)
+++ gcc/gimple-ssa-store-merging.c      (working copy)
@@ -140,19 +140,17 @@ struct store_immediate_info
 {
   unsigned HOST_WIDE_INT bitsize;
   unsigned HOST_WIDE_INT bitpos;
-  tree val;
-  tree dest;
   gimple *stmt;
   unsigned int order;
-  store_immediate_info (unsigned HOST_WIDE_INT, unsigned HOST_WIDE_INT, tree,
-                       tree, gimple *, unsigned int);
+  store_immediate_info (unsigned HOST_WIDE_INT, unsigned HOST_WIDE_INT,
+                       gimple *, unsigned int);
 };
 
 store_immediate_info::store_immediate_info (unsigned HOST_WIDE_INT bs,
-                                           unsigned HOST_WIDE_INT bp, tree v,
-                                           tree d, gimple *st,
+                                           unsigned HOST_WIDE_INT bp,
+                                           gimple *st,
                                            unsigned int ord)
-  : bitsize (bs), bitpos (bp), val (v), dest (d), stmt (st), order (ord)
+  : bitsize (bs), bitpos (bp), stmt (st), order (ord)
 {
 }
 
@@ -557,7 +555,7 @@ merged_store_group::merged_store_group (
   /* VAL has memory allocated for it in apply_stores once the group
      width has been finalized.  */
   val = NULL;
-  align = get_object_alignment (info->dest);
+  align = get_object_alignment (gimple_assign_lhs (info->stmt));
   stores.create (1);
   stores.safe_push (info);
   last_stmt = info->stmt;
@@ -654,14 +652,16 @@ merged_store_group::apply_stores ()
   FOR_EACH_VEC_ELT (stores, i, info)
     {
       unsigned int pos_in_buffer = info->bitpos - start;
-      bool ret = encode_tree_to_bitpos (info->val, val, info->bitsize,
-                                        pos_in_buffer, buf_size);
+      bool ret = encode_tree_to_bitpos (gimple_assign_rhs1 (info->stmt),
+                                       val, info->bitsize,
+                                       pos_in_buffer, buf_size);
       if (dump_file && (dump_flags & TDF_DETAILS))
        {
          if (ret)
            {
              fprintf (dump_file, "After writing ");
-             print_generic_expr (dump_file, info->val, 0);
+             print_generic_expr (dump_file,
+                                 gimple_assign_rhs1 (info->stmt), 0);
              fprintf (dump_file, " of size " HOST_WIDE_INT_PRINT_DEC
                        " at position %d the merged region contains:\n",
                        info->bitsize, pos_in_buffer);
@@ -680,13 +680,15 @@ merged_store_group::apply_stores ()
 
 struct imm_store_chain_info
 {
+  tree base_addr;
   auto_vec<struct store_immediate_info *> m_store_info;
   auto_vec<merged_store_group *> m_merged_store_groups;
 
-  bool terminate_and_process_chain (tree);
+  imm_store_chain_info (tree b_a) : base_addr (b_a) {}
+  bool terminate_and_process_chain ();
   bool coalesce_immediate_stores ();
-  bool output_merged_store (tree, merged_store_group *);
-  bool output_merged_stores (tree);
+  bool output_merged_store (merged_store_group *);
+  bool output_merged_stores ();
 };
 
 const pass_data pass_data_tree_store_merging = {
@@ -722,8 +724,9 @@ private:
   hash_map<tree_operand_hash, struct imm_store_chain_info *> m_stores;
 
   bool terminate_and_process_all_chains ();
-  bool terminate_all_aliasing_chains (tree, tree, bool, gimple *);
-  bool terminate_and_release_chain (tree);
+  bool terminate_all_aliasing_chains (tree, imm_store_chain_info **,
+                                     bool, gimple *);
+  bool terminate_and_release_chain (imm_store_chain_info *);
 }; // class pass_store_merging
 
 /* Terminate and process all recorded chains.  Return true if any changes
@@ -736,7 +739,7 @@ pass_store_merging::terminate_and_proces
     = m_stores.begin ();
   bool ret = false;
   for (; iter != m_stores.end (); ++iter)
-    ret |= terminate_and_release_chain ((*iter).first);
+    ret |= terminate_and_release_chain ((*iter).second);
 
   return ret;
 }
@@ -750,7 +753,9 @@ pass_store_merging::terminate_and_proces
    If that is the case we have to terminate any chain anchored at BASE.  */
 
 bool
-pass_store_merging::terminate_all_aliasing_chains (tree dest, tree base,
+pass_store_merging::terminate_all_aliasing_chains (tree dest,
+                                                  imm_store_chain_info
+                                                    **chain_info,
                                                   bool var_offset_p,
                                                   gimple *stmt)
 {
@@ -760,44 +765,38 @@ pass_store_merging::terminate_all_aliasi
   if (!gimple_vuse (stmt))
     return false;
 
-  struct imm_store_chain_info **chain_info = NULL;
-
   /* Check if the assignment destination (BASE) is part of a store chain.
      This is to catch non-constant stores to destinations that may be part
      of a chain.  */
-  if (base)
+  if (chain_info)
     {
-      chain_info = m_stores.get (base);
-      if (chain_info)
+      /* We have a chain at BASE and we're writing to [BASE + <variable>].
+        This can interfere with any of the stores so terminate
+        the chain.  */
+      if (var_offset_p)
        {
-         /* We have a chain at BASE and we're writing to [BASE + <variable>].
-            This can interfere with any of the stores so terminate
-            the chain.  */
-         if (var_offset_p)
-           {
-             terminate_and_release_chain (base);
-             ret = true;
-           }
-         /* Otherwise go through every store in the chain to see if it
-            aliases with any of them.  */
-         else
+         terminate_and_release_chain (*chain_info);
+         ret = true;
+       }
+      /* Otherwise go through every store in the chain to see if it
+        aliases with any of them.  */
+      else
+       {
+         struct store_immediate_info *info;
+         unsigned int i;
+         FOR_EACH_VEC_ELT ((*chain_info)->m_store_info, i, info)
            {
-             struct store_immediate_info *info;
-             unsigned int i;
-             FOR_EACH_VEC_ELT ((*chain_info)->m_store_info, i, info)
+             if (stmt_may_clobber_ref_p (info->stmt, dest))
                {
-                 if (refs_may_alias_p (info->dest, dest))
+                 if (dump_file && (dump_flags & TDF_DETAILS))
                    {
-                     if (dump_file && (dump_flags & TDF_DETAILS))
-                       {
-                         fprintf (dump_file,
-                                  "stmt causes chain termination:\n");
-                         print_gimple_stmt (dump_file, stmt, 0, 0);
-                       }
-                     terminate_and_release_chain (base);
-                     ret = true;
-                     break;
+                     fprintf (dump_file,
+                              "stmt causes chain termination:\n");
+                     print_gimple_stmt (dump_file, stmt, 0, 0);
                    }
+                 terminate_and_release_chain (*chain_info);
+                 ret = true;
+                 break;
                }
            }
        }
@@ -814,11 +813,16 @@ pass_store_merging::terminate_all_aliasi
       if (chain_info && (*chain_info) == (*iter).second)
        continue;
 
-      tree key = (*iter).first;
-      if (ref_maybe_used_by_stmt_p (stmt, key)
-         || stmt_may_clobber_ref_p (stmt, key))
+      /* We can't use the base object here as that does not reliably exist.
+        Build a ao_ref from the base object address (if we know the
+        minimum and maximum offset and the maximum size we could improve
+        things here).  */
+      ao_ref chain_ref;
+      ao_ref_init_from_ptr_and_size (&chain_ref, (*iter).first, NULL_TREE);
+      if (ref_maybe_used_by_stmt_p (stmt, &chain_ref)
+         || stmt_may_clobber_ref_p_1 (stmt, &chain_ref))
        {
-         terminate_and_release_chain (key);
+         terminate_and_release_chain ((*iter).second);
          ret = true;
        }
     }
@@ -831,19 +835,11 @@ pass_store_merging::terminate_all_aliasi
    entry is removed after the processing in any case.  */
 
 bool
-pass_store_merging::terminate_and_release_chain (tree base)
+pass_store_merging::terminate_and_release_chain (imm_store_chain_info 
*chain_info)
 {
-  struct imm_store_chain_info **chain_info = m_stores.get (base);
-
-  if (!chain_info)
-    return false;
-
-  gcc_assert (*chain_info);
-
-  bool ret = (*chain_info)->terminate_and_process_chain (base);
-  delete *chain_info;
-  m_stores.remove (base);
-
+  bool ret = chain_info->terminate_and_process_chain ();
+  m_stores.remove (chain_info->base_addr);
+  delete chain_info;
   return ret;
 }
 
@@ -880,7 +876,7 @@ imm_store_chain_info::coalesce_immediate
          fprintf (dump_file, "Store %u:\nbitsize:" HOST_WIDE_INT_PRINT_DEC
                              " bitpos:" HOST_WIDE_INT_PRINT_DEC " val:\n",
                   i, info->bitsize, info->bitpos);
-         print_generic_expr (dump_file, info->val, 0);
+         print_generic_expr (dump_file, gimple_assign_rhs1 (info->stmt), 0);
          fprintf (dump_file, "\n------------\n");
        }
 
@@ -1103,7 +1099,7 @@ split_group (merged_store_group *group,
    return true.  */
 
 bool
-imm_store_chain_info::output_merged_store (tree base, merged_store_group 
*group)
+imm_store_chain_info::output_merged_store (merged_store_group *group)
 {
   unsigned HOST_WIDE_INT start_byte_pos = group->start / BITS_PER_UNIT;
 
@@ -1141,7 +1137,7 @@ imm_store_chain_info::output_merged_stor
 
       tree int_type = build_nonstandard_integer_type (try_size, UNSIGNED);
       int_type = build_aligned_type (int_type, align);
-      tree addr = build_fold_addr_expr (base);
+      tree addr = build_fold_addr_expr (base_addr);
       tree dest = fold_build2 (MEM_REF, int_type, addr,
                               build_int_cst (offset_type, try_pos));
 
@@ -1213,14 +1209,14 @@ imm_store_chain_info::output_merged_stor
    successful.  Return true iff any changes were made.  */
 
 bool
-imm_store_chain_info::output_merged_stores (tree base)
+imm_store_chain_info::output_merged_stores ()
 {
   unsigned int i;
   merged_store_group *merged_store;
   bool ret = false;
   FOR_EACH_VEC_ELT (m_merged_store_groups, i, merged_store)
     {
-      if (output_merged_store (base, merged_store))
+      if (output_merged_store (merged_store))
        {
          unsigned int j;
          store_immediate_info *store;
@@ -1250,7 +1246,7 @@ imm_store_chain_info::output_merged_stor
    Return true if any changes were made.  */
 
 bool
-imm_store_chain_info::terminate_and_process_chain (tree base)
+imm_store_chain_info::terminate_and_process_chain ()
 {
   /* Process store chain.  */
   bool ret = false;
@@ -1258,7 +1254,7 @@ imm_store_chain_info::terminate_and_proc
     {
       ret = coalesce_immediate_stores ();
       if (ret)
-       ret = output_merged_stores (base);
+       ret = output_merged_stores ();
     }
 
   /* Delete all the entries we allocated ourselves.  */
@@ -1413,7 +1409,7 @@ pass_store_merging::execute (function *f
                  if (chain_info)
                    {
                      info = new store_immediate_info (
-                       bitsize, bitpos, rhs, lhs, stmt,
+                       bitsize, bitpos, stmt,
                        (*chain_info)->m_store_info.length ());
                      if (dump_file && (dump_flags & TDF_DETAILS))
                        {
@@ -1432,17 +1428,17 @@ pass_store_merging::execute (function *f
                            fprintf (dump_file,
                                 "Reached maximum number of statements"
                                 " to merge:\n");
-                         terminate_and_release_chain (base_addr);
+                         terminate_and_release_chain (*chain_info);
                        }
                      continue;
                    }
 
                  /* Store aliases any existing chain?  */
-                 terminate_all_aliasing_chains (lhs, base_addr, false, stmt);
+                 terminate_all_aliasing_chains (lhs, chain_info, false, stmt);
                  /* Start a new chain.  */
                  struct imm_store_chain_info *new_chain
-                   = new imm_store_chain_info;
-                 info = new store_immediate_info (bitsize, bitpos, rhs, lhs,
+                   = new imm_store_chain_info (base_addr);
+                 info = new store_immediate_info (bitsize, bitpos,
                                                   stmt, 0);
                  new_chain->m_store_info.safe_push (info);
                  m_stores.put (base_addr, new_chain);
@@ -1457,13 +1453,13 @@ pass_store_merging::execute (function *f
                    }
                }
              else
-               terminate_all_aliasing_chains (lhs, base_addr,
+               terminate_all_aliasing_chains (lhs, chain_info,
                                               offset != NULL_TREE, stmt);
 
              continue;
            }
 
-         terminate_all_aliasing_chains (NULL_TREE, NULL_TREE, false, stmt);
+         terminate_all_aliasing_chains (NULL_TREE, NULL, false, stmt);
        }
       terminate_and_process_all_chains ();
     }

Reply via email to