The following puts a limit on the number of alias tests we do in
terminate_all_aliasing_chains which is quadratic in the number of
overall stores currentrly tracked.  There is already a limit in
place on the maximum number of stores in a single chain so the
following adds a limit on the number of chains tracked.  The
worst number of overall stores tracked from the defaults (64 and 64)
is then 4096 which when imposed as the sole limit for the testcase
still causes

 store merging                      :  71.65 ( 56%)

because the testcase is somewhat degenerate with most chains
consisting only of a single store (and 25% of exactly three stores).
The single stores are all CLOBBERs at the point variables go out of
scope.  Note unpatched we have

 store merging                      : 308.60 ( 84%)

Limiting the number of chains to 64 brings this down to

 store merging                      :   1.52 (  3%)

which is more reasonable.  There are ideas on how to make
terminate_all_aliasing_chains cheaper but for this degenerate case
they would not have any effect so I'll defer for GCC 12 for those.

I'm not sure we want to have both --params, just keeping the
more to-the-point max-stores-to-track works but makes the
degenerate case above slower.
I made the current default 1024 (a million alias disambiguations)
which for the testcasse (without limiting chains) results in 25%
compile time and 20s putting it in the same ballpart as the next
offender (which is PTA).

This is a regression on trunk and the GCC 10 branch btw.

Bootstrapped and tested on x86_64-unknown-linux-gnu.

Any opinions?

Thanks,
Richard.

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

        PR tree-optimization/38474
        * params.opt (-param=max-store-chains-to-track=): New param.
        (-param=max-stores-to-track=): Likewise.
        * doc/invoke.texi (max-store-chains-to-track): Document.
        (max-stores-to-track): Likewise.
        * gimple-ssa-store-merging.c (pass_store_merging::m_n_chains):
        New.
        (pass_store_merging::m_n_stores): Likewise.
        (pass_store_merging::terminate_and_process_chain): Update
        m_n_stores and m_n_chains.
        (pass_store_merging::process_store): Likewise.   Terminate
        oldest chains if the number of stores or chains get too large.
        (imm_store_chain_info::terminate_and_process_chain): Dump
        chain length.
---
 gcc/doc/invoke.texi            |  8 ++++
 gcc/gimple-ssa-store-merging.c | 88 ++++++++++++++++++++++++++--------
 gcc/params.opt                 |  8 ++++
 3 files changed, 83 insertions(+), 21 deletions(-)

diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index 3751bc3ac7c..e8baa545eee 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -13308,6 +13308,14 @@ do so.
 The maximum number of stores to attempt to merge into wider stores in the store
 merging pass.
 
+@item max-store-chains-to-track
+The maximum number of store chains to track at the same time in the attempt
+to merge them into wider stores in the store merging pass.
+
+@item max-stores-to-track
+The maximum number of stores to track at the same time in the attemt to
+to merge them into wider stores in the store merging pass.
+
 @item max-unrolled-insns
 The maximum number of instructions that a loop may have to be unrolled.
 If a loop is unrolled, this parameter also determines how many times
diff --git a/gcc/gimple-ssa-store-merging.c b/gcc/gimple-ssa-store-merging.c
index f0f4a068de5..b4c5e8eb9a8 100644
--- a/gcc/gimple-ssa-store-merging.c
+++ b/gcc/gimple-ssa-store-merging.c
@@ -2324,7 +2324,8 @@ class pass_store_merging : public gimple_opt_pass
 {
 public:
   pass_store_merging (gcc::context *ctxt)
-    : gimple_opt_pass (pass_data_tree_store_merging, ctxt), m_stores_head ()
+    : gimple_opt_pass (pass_data_tree_store_merging, ctxt), m_stores_head (),
+      m_n_chains (0), m_n_stores (0)
   {
   }
 
@@ -2356,6 +2357,11 @@ private:
      decisions when going out of SSA).  */
   imm_store_chain_info *m_stores_head;
 
+  /* The number of store chains currently tracked.  */
+  unsigned m_n_chains;
+  /* The number of stores currently tracked.  */
+  unsigned m_n_stores;
+
   bool process_store (gimple *);
   bool terminate_and_process_chain (imm_store_chain_info *);
   bool terminate_all_aliasing_chains (imm_store_chain_info **, gimple *);
@@ -2435,6 +2441,8 @@ pass_store_merging::terminate_all_aliasing_chains 
(imm_store_chain_info
 bool
 pass_store_merging::terminate_and_process_chain (imm_store_chain_info 
*chain_info)
 {
+  m_n_stores -= chain_info->m_store_info.length ();
+  m_n_chains--;
   bool ret = chain_info->terminate_and_process_chain ();
   m_stores.remove (chain_info->base_addr);
   delete chain_info;
@@ -4711,6 +4719,9 @@ imm_store_chain_info::output_merged_stores ()
 bool
 imm_store_chain_info::terminate_and_process_chain ()
 {
+  if (dump_file && (dump_flags & TDF_DETAILS))
+    fprintf (dump_file, "Terminating chain with %u stores\n",
+            m_store_info.length ());
   /* Process store chain.  */
   bool ret = false;
   if (m_store_info.length () > 1)
@@ -5159,6 +5170,7 @@ pass_store_merging::process_store (gimple *stmt)
          print_gimple_stmt (dump_file, stmt, 0);
        }
       (*chain_info)->m_store_info.safe_push (info);
+      m_n_stores++;
       ret |= terminate_all_aliasing_chains (chain_info, stmt);
       /* If we reach the limit of stores to merge in a chain terminate and
         process the chain now.  */
@@ -5170,30 +5182,64 @@ pass_store_merging::process_store (gimple *stmt)
                     "Reached maximum number of statements to merge:\n");
          ret |= terminate_and_process_chain (*chain_info);
        }
-      return ret;
     }
+  else
+    {
+      /* Store aliases any existing chain?  */
+      ret |= terminate_all_aliasing_chains (NULL, stmt);
 
-  /* Store aliases any existing chain?  */
-  ret |= terminate_all_aliasing_chains (NULL, stmt);
-  /* Start a new chain.  */
-  class imm_store_chain_info *new_chain
-    = new imm_store_chain_info (m_stores_head, base_addr);
-  info = new store_immediate_info (const_bitsize, const_bitpos,
-                                  const_bitregion_start,
-                                  const_bitregion_end,
-                                  stmt, 0, rhs_code, n, ins_stmt,
-                                  bit_not_p, lp_nr_for_store (stmt),
-                                  ops[0], ops[1]);
-  new_chain->m_store_info.safe_push (info);
-  m_stores.put (base_addr, new_chain);
-  if (dump_file && (dump_flags & TDF_DETAILS))
+      /* Start a new chain.  */
+      class imm_store_chain_info *new_chain
+         = new imm_store_chain_info (m_stores_head, base_addr);
+      info = new store_immediate_info (const_bitsize, const_bitpos,
+                                      const_bitregion_start,
+                                      const_bitregion_end,
+                                      stmt, 0, rhs_code, n, ins_stmt,
+                                      bit_not_p, lp_nr_for_store (stmt),
+                                      ops[0], ops[1]);
+      new_chain->m_store_info.safe_push (info);
+      m_n_stores++;
+      m_stores.put (base_addr, new_chain);
+      m_n_chains++;
+      if (dump_file && (dump_flags & TDF_DETAILS))
+       {
+         fprintf (dump_file, "Starting active chain number %u with 
statement:\n",
+                  m_n_chains);
+         print_gimple_stmt (dump_file, stmt, 0);
+         fprintf (dump_file, "The base object is:\n");
+         print_generic_expr (dump_file, base_addr);
+         fprintf (dump_file, "\n");
+       }
+    }
+
+  /* Prune oldest chains so that after adding the chain or store above
+     we're again within the limits set by the params.  */
+  if (m_n_chains > (unsigned)param_max_store_chains_to_track
+      || m_n_stores > (unsigned)param_max_stores_to_track)
     {
-      fprintf (dump_file, "Starting new chain with statement:\n");
-      print_gimple_stmt (dump_file, stmt, 0);
-      fprintf (dump_file, "The base object is:\n");
-      print_generic_expr (dump_file, base_addr);
-      fprintf (dump_file, "\n");
+      if (dump_file && (dump_flags & TDF_DETAILS))
+       fprintf (dump_file, "Too many chains (%u > %d) or stores (%u > %d), "
+                "terminating oldest chain(s).\n", m_n_chains,
+                param_max_store_chains_to_track, m_n_stores,
+                param_max_stores_to_track);
+      imm_store_chain_info **e = &m_stores_head;
+      unsigned idx = 0;
+      unsigned n_stores = 0;
+      while (*e)
+       {
+         if (idx >= (unsigned)param_max_store_chains_to_track
+             || (n_stores + (*e)->m_store_info.length ()
+                 > (unsigned)param_max_stores_to_track))
+           terminate_and_process_chain (*e);
+         else
+           {
+             n_stores += (*e)->m_store_info.length ();
+             e = &(*e)->next;
+             ++idx;
+           }
+       }
     }
+
   return ret;
 }
 
diff --git a/gcc/params.opt b/gcc/params.opt
index dff8a331f82..c633648d047 100644
--- a/gcc/params.opt
+++ b/gcc/params.opt
@@ -681,6 +681,14 @@ Maximum number of constant stores to merge in the store 
merging pass.
 Common Joined UInteger Var(param_max_stores_to_sink) Init(2) Param Optimization
 Maximum number of conditional store pairs that can be sunk.
 
+-param=max-store-chains-to-track=
+Common Joined UInteger Var(param_max_store_chains_to_track) Init(64) 
IntegerRange(1, 65536)
+Maximum number of store chains to track at the same time in the store merging 
pass.
+
+-param=max-stores-to-track=
+Common Joined UInteger Var(param_max_stores_to_track) Init(1024) 
IntegerRange(2, 1048576)
+Maximum number of store chains to track at the same time in the store merging 
pass.
+
 -param=max-tail-merge-comparisons=
 Common Joined UInteger Var(param_max_tail_merge_comparisons) Init(10) Param 
Optimization
 Maximum amount of similar bbs to compare a bb with.
-- 
2.26.2

Reply via email to