tree-optimization/38474 - fix store-merging compile-time regression
authorRichard Biener <rguenther@suse.de>
Thu, 11 Feb 2021 10:13:47 +0000 (11:13 +0100)
committerRichard Biener <rguenther@suse.de>
Fri, 12 Feb 2021 08:38:52 +0000 (09:38 +0100)
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 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.

2021-02-11  Richard Biener  <rguenther@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
gcc/gimple-ssa-store-merging.c
gcc/params.opt

index 3751bc3ac7c56c5e198a01b3150ba6b15b6a9722..e8baa545eee95bfa43111b61acbcbb280ae86f29 100644 (file)
@@ -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
index f0f4a068de583b1cd0a9230e7ad5aeb88ef5a4a7..b4c5e8eb9a8b6d3576c0fdbe069957474ded90ad 100644 (file)
@@ -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;
 }
 
index dff8a331f82e44c1b90e5b9f88ffc61e84f03d2d..c633648d0472264c1be7a21f2e4d7d4134cf86b8 100644 (file)
@@ -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.