From 95d94b52ea8478334fb92cca545f0bd904bd0034 Mon Sep 17 00:00:00 2001 From: Richard Biener Date: Thu, 11 Feb 2021 11:13:47 +0100 Subject: [PATCH] tree-optimization/38474 - fix store-merging compile-time regression 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 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.30.2