From patchwork Sun Oct 25 13:22:48 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jan Hubicka X-Patchwork-Id: 1387232 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=gcc.gnu.org (client-ip=8.43.85.97; helo=sourceware.org; envelope-from=gcc-patches-bounces@gcc.gnu.org; receiver=) Authentication-Results: ozlabs.org; dmarc=none (p=none dis=none) header.from=ucw.cz Received: from sourceware.org (server2.sourceware.org [8.43.85.97]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 4CJzDN0Gmfz9sSG for ; Mon, 26 Oct 2020 00:22:58 +1100 (AEDT) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 89E57385700D; Sun, 25 Oct 2020 13:22:55 +0000 (GMT) X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from nikam.ms.mff.cuni.cz (nikam.ms.mff.cuni.cz [195.113.20.16]) by sourceware.org (Postfix) with ESMTPS id 484103857C68 for ; Sun, 25 Oct 2020 13:22:51 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 484103857C68 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=ucw.cz Authentication-Results: sourceware.org; spf=none smtp.mailfrom=hubicka@kam.mff.cuni.cz Received: by nikam.ms.mff.cuni.cz (Postfix, from userid 16202) id C6AFE282622; Sun, 25 Oct 2020 14:22:48 +0100 (CET) Date: Sun, 25 Oct 2020 14:22:48 +0100 From: Jan Hubicka To: gcc-patches@gcc.gnu.org, mliska@suse.cz, mjambor@suse.cz Subject: Make default duplicate and insert methods of summaries abort; fix fallout Message-ID: <20201025132248.GE46434@kam.mff.cuni.cz> MIME-Version: 1.0 Content-Disposition: inline User-Agent: Mutt/1.10.1 (2018-07-13) X-Spam-Status: No, score=-15.9 required=5.0 tests=BAYES_00, GIT_PATCH_0, HEADER_FROM_DIFFERENT_DOMAINS, KAM_ASCII_DIVIDERS, KAM_DMARC_STATUS, KAM_LAZY_DOMAIN_SECURITY, RCVD_IN_MSPIKE_H3, RCVD_IN_MSPIKE_WL, SPF_HELO_NONE, SPF_NONE, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: gcc-patches-bounces@gcc.gnu.org Sender: "Gcc-patches" Hi, the default duplicate and insert methods of sumaries produce empty summary that is not useful for anything and makes it easy to introduce bugs. This patch makes the default hooks to abort and summaries that do not need dupicaito/insertion disable the corresponding hooks. I also implemented missing insertion hook for ipa-sra which forced me to move analysis out of anounmous namespace. Wi aready have disable_insertion_hook, I also added disable_duplication_hook. Martin (Liska), it would be nice to simply unregiter the hooks instead of having bool controlling htem so we save some indirect calls. Bootstrapped/regtested x86_64-linux, plan to commit it tomorrow if there are no comments. Honza 2020-10-23 Jan Hubicka * cgraph.h (struct cgraph_node): Make ipa_transforms_to_apply vl_ptr. * ipa-inline-analysis.c (initialize_growth_caches): Disable insertion and duplication hooks. * ipa-inline-transform.c (clone_inlined_nodes): Clear ipa_transforms_to_apply. (save_inline_function_body): Disable insertion hoook for ipa_saved_clone_sources. * ipa-prop.c (ipcp_transformation_initialize): Disable insertion hook. * ipa-prop.h (ipa_node_params_t): Disable insertion hook. * ipa-reference.c (propagate): Disable insertion hoook. * ipa-sra.c (ipa_sra_summarize_function): Move out of anonymous namespace. (ipa_sra_function_summaries::insert): New virtual function. * passes.c (execute_one_pass): Do not add transforms to inline clones. * symbol-summary.h (function_summary_base): Make insert and duplicate hooks fail instead of silently producing empty summaries; add way to disable duplication hooks (call_summary_base): Likewise. * tree-nested.c (nested_function_info::get_create): Disable insertion hooks (maybe_record_nested_function): Likewise. diff --git a/gcc/cgraph.h b/gcc/cgraph.h index 9eb48d5b62f..65e4646efcd 100644 --- a/gcc/cgraph.h +++ b/gcc/cgraph.h @@ -1402,7 +1402,7 @@ struct GTY((tag ("SYMTAB_FUNCTION"))) cgraph_node : public symtab_node /* Interprocedural passes scheduled to have their transform functions applied next time we execute local pass on them. We maintain it per-function in order to allow IPA passes to introduce new functions. */ - vec GTY((skip)) ipa_transforms_to_apply; + vec GTY((skip)) ipa_transforms_to_apply; /* For inline clones this points to the function they will be inlined into. */ diff --git a/gcc/ipa-inline-analysis.c b/gcc/ipa-inline-analysis.c index acbf82e84d9..bd0e322605f 100644 --- a/gcc/ipa-inline-analysis.c +++ b/gcc/ipa-inline-analysis.c @@ -127,6 +127,9 @@ initialize_growth_caches () = new fast_call_summary (symtab); node_context_cache = new fast_function_summary (symtab); + edge_growth_cache->disable_duplication_hook (); + node_context_cache->disable_insertion_hook (); + node_context_cache->disable_duplication_hook (); } /* Free growth caches. */ diff --git a/gcc/ipa-inline-transform.c b/gcc/ipa-inline-transform.c index 3782cce12e3..279ba2f7cb0 100644 --- a/gcc/ipa-inline-transform.c +++ b/gcc/ipa-inline-transform.c @@ -231,6 +231,11 @@ clone_inlined_nodes (struct cgraph_edge *e, bool duplicate, e->callee->remove_from_same_comdat_group (); e->callee->inlined_to = inlining_into; + if (e->callee->ipa_transforms_to_apply.length ()) + { + e->callee->ipa_transforms_to_apply.release (); + e->callee->ipa_transforms_to_apply = vNULL; + } /* Recursively clone all bodies. */ for (e = e->callee->callees; e; e = next) @@ -606,7 +611,10 @@ save_inline_function_body (struct cgraph_node *node) tree prev_body_holder = node->decl; if (!ipa_saved_clone_sources) - ipa_saved_clone_sources = new function_summary (symtab); + { + ipa_saved_clone_sources = new function_summary (symtab); + ipa_saved_clone_sources->disable_insertion_hook (); + } else { tree *p = ipa_saved_clone_sources->get (node); diff --git a/gcc/ipa-prop.c b/gcc/ipa-prop.c index a848f1db95e..6014766b418 100644 --- a/gcc/ipa-prop.c +++ b/gcc/ipa-prop.c @@ -4211,7 +4211,10 @@ ipcp_transformation_initialize (void) if (!ipa_vr_hash_table) ipa_vr_hash_table = hash_table::create_ggc (37); if (ipcp_transformation_sum == NULL) - ipcp_transformation_sum = ipcp_transformation_t::create_ggc (symtab); + { + ipcp_transformation_sum = ipcp_transformation_t::create_ggc (symtab); + ipcp_transformation_sum->disable_insertion_hook (); + } } /* Release the IPA CP transformation summary. */ diff --git a/gcc/ipa-prop.h b/gcc/ipa-prop.h index 0bbbbf9bd9f..77e92b04bba 100644 --- a/gcc/ipa-prop.h +++ b/gcc/ipa-prop.h @@ -941,7 +941,10 @@ class GTY((user)) ipa_node_params_t: public function_summary { public: ipa_node_params_t (symbol_table *table, bool ggc): - function_summary (table, ggc) { } + function_summary (table, ggc) + { + disable_insertion_hook (); + } /* Hook that is called by summary when a node is duplicated. */ virtual void duplicate (cgraph_node *node, diff --git a/gcc/ipa-reference.c b/gcc/ipa-reference.c index 4a6c011c525..ced83d218ab 100644 --- a/gcc/ipa-reference.c +++ b/gcc/ipa-reference.c @@ -894,7 +894,10 @@ propagate (void) } if (ipa_ref_opt_sum_summaries == NULL) - ipa_ref_opt_sum_summaries = new ipa_ref_opt_summary_t (symtab); + { + ipa_ref_opt_sum_summaries = new ipa_ref_opt_summary_t (symtab); + ipa_ref_opt_sum_summaries->disable_insertion_hook (); + } /* Cleanup. */ FOR_EACH_DEFINED_FUNCTION (node) diff --git a/gcc/ipa-sra.c b/gcc/ipa-sra.c index 8d4f580e009..07227c0bfec 100644 --- a/gcc/ipa-sra.c +++ b/gcc/ipa-sra.c @@ -85,6 +85,8 @@ along with GCC; see the file COPYING3. If not see #include "tree-streamer.h" #include "internal-fn.h" +static void ipa_sra_summarize_function (cgraph_node *); + /* Bits used to track size of an aggregate in bytes interprocedurally. */ #define ISRA_ARG_SIZE_LIMIT_BITS 16 #define ISRA_ARG_SIZE_LIMIT (1 << ISRA_ARG_SIZE_LIMIT_BITS) @@ -373,6 +375,7 @@ public: virtual void duplicate (cgraph_node *, cgraph_node *, isra_func_summary *old_sum, isra_func_summary *new_sum); + virtual void insert (cgraph_node *, isra_func_summary *); }; /* Hook that is called by summary when a node is duplicated. */ @@ -426,6 +429,21 @@ ipa_sra_function_summaries::duplicate (cgraph_node *, cgraph_node *, static GTY(()) ipa_sra_function_summaries *func_sums; +/* Hook that is called by summary when new node appears. */ + +void +ipa_sra_function_summaries::insert (cgraph_node *node, isra_func_summary *) +{ + if (opt_for_fn (node->decl, flag_ipa_sra)) + { + push_cfun (DECL_STRUCT_FUNCTION (node->decl)); + ipa_sra_summarize_function (node); + pop_cfun (); + } + else + func_sums->remove (node); +} + /* Class to manage call summaries. */ class ipa_sra_call_summaries: public call_summary @@ -2478,79 +2496,6 @@ verify_splitting_accesses (cgraph_node *node, bool certain_must_exist) } } -/* Intraprocedural part of IPA-SRA analysis. Scan function body of NODE and - create a summary structure describing IPA-SRA opportunities and constraints - in it. */ - -static void -ipa_sra_summarize_function (cgraph_node *node) -{ - if (dump_file) - fprintf (dump_file, "Creating summary for %s/%i:\n", node->name (), - node->order); - if (!ipa_sra_preliminary_function_checks (node)) - return; - gcc_obstack_init (&gensum_obstack); - isra_func_summary *ifs = func_sums->get_create (node); - ifs->m_candidate = true; - tree ret = TREE_TYPE (TREE_TYPE (node->decl)); - ifs->m_returns_value = (TREE_CODE (ret) != VOID_TYPE); - - decl2desc = new hash_map; - unsigned count = 0; - for (tree parm = DECL_ARGUMENTS (node->decl); parm; parm = DECL_CHAIN (parm)) - count++; - - if (count > 0) - { - auto_vec param_descriptions (count); - param_descriptions.reserve_exact (count); - param_descriptions.quick_grow_cleared (count); - - bool cfun_pushed = false; - struct function *fun = DECL_STRUCT_FUNCTION (node->decl); - if (create_parameter_descriptors (node, ¶m_descriptions)) - { - push_cfun (fun); - cfun_pushed = true; - final_bbs = BITMAP_ALLOC (NULL); - bb_dereferences = XCNEWVEC (HOST_WIDE_INT, - by_ref_count - * last_basic_block_for_fn (fun)); - aa_walking_limit = opt_for_fn (node->decl, param_ipa_max_aa_steps); - scan_function (node, fun); - - if (dump_file) - { - dump_gensum_param_descriptors (dump_file, node->decl, - ¶m_descriptions); - fprintf (dump_file, "----------------------------------------\n"); - } - } - process_scan_results (node, fun, ifs, ¶m_descriptions); - - if (cfun_pushed) - pop_cfun (); - if (bb_dereferences) - { - free (bb_dereferences); - bb_dereferences = NULL; - BITMAP_FREE (final_bbs); - final_bbs = NULL; - } - } - isra_analyze_all_outgoing_calls (node); - - delete decl2desc; - decl2desc = NULL; - obstack_free (&gensum_obstack, NULL); - if (dump_file) - fprintf (dump_file, "\n\n"); - if (flag_checking) - verify_splitting_accesses (node, false); - return; -} - /* Intraprocedural part of IPA-SRA analysis. Scan bodies of all functions in this compilation unit and create summary structures describing IPA-SRA opportunities and constraints in them. */ @@ -4102,6 +4047,79 @@ public: } // anon namespace +/* Intraprocedural part of IPA-SRA analysis. Scan function body of NODE and + create a summary structure describing IPA-SRA opportunities and constraints + in it. */ + +static void +ipa_sra_summarize_function (cgraph_node *node) +{ + if (dump_file) + fprintf (dump_file, "Creating summary for %s/%i:\n", node->name (), + node->order); + if (!ipa_sra_preliminary_function_checks (node)) + return; + gcc_obstack_init (&gensum_obstack); + isra_func_summary *ifs = func_sums->get_create (node); + ifs->m_candidate = true; + tree ret = TREE_TYPE (TREE_TYPE (node->decl)); + ifs->m_returns_value = (TREE_CODE (ret) != VOID_TYPE); + + decl2desc = new hash_map; + unsigned count = 0; + for (tree parm = DECL_ARGUMENTS (node->decl); parm; parm = DECL_CHAIN (parm)) + count++; + + if (count > 0) + { + auto_vec param_descriptions (count); + param_descriptions.reserve_exact (count); + param_descriptions.quick_grow_cleared (count); + + bool cfun_pushed = false; + struct function *fun = DECL_STRUCT_FUNCTION (node->decl); + if (create_parameter_descriptors (node, ¶m_descriptions)) + { + push_cfun (fun); + cfun_pushed = true; + final_bbs = BITMAP_ALLOC (NULL); + bb_dereferences = XCNEWVEC (HOST_WIDE_INT, + by_ref_count + * last_basic_block_for_fn (fun)); + aa_walking_limit = opt_for_fn (node->decl, param_ipa_max_aa_steps); + scan_function (node, fun); + + if (dump_file) + { + dump_gensum_param_descriptors (dump_file, node->decl, + ¶m_descriptions); + fprintf (dump_file, "----------------------------------------\n"); + } + } + process_scan_results (node, fun, ifs, ¶m_descriptions); + + if (cfun_pushed) + pop_cfun (); + if (bb_dereferences) + { + free (bb_dereferences); + bb_dereferences = NULL; + BITMAP_FREE (final_bbs); + final_bbs = NULL; + } + } + isra_analyze_all_outgoing_calls (node); + + delete decl2desc; + decl2desc = NULL; + obstack_free (&gensum_obstack, NULL); + if (dump_file) + fprintf (dump_file, "\n\n"); + if (flag_checking) + verify_splitting_accesses (node, false); + return; +} + ipa_opt_pass_d * make_pass_ipa_sra (gcc::context *ctxt) { diff --git a/gcc/passes.c b/gcc/passes.c index 1942b7cd1c3..02a47e2595c 100644 --- a/gcc/passes.c +++ b/gcc/passes.c @@ -2566,7 +2566,8 @@ execute_one_pass (opt_pass *pass) { struct cgraph_node *node; FOR_EACH_FUNCTION_WITH_GIMPLE_BODY (node) - node->ipa_transforms_to_apply.safe_push ((ipa_opt_pass_d *)pass); + if (!node->inlined_to) + node->ipa_transforms_to_apply.safe_push ((ipa_opt_pass_d *)pass); } else if (dump_file) do_per_function (execute_function_dump, pass); diff --git a/gcc/symbol-summary.h b/gcc/symbol-summary.h index a38eb1db778..af5f4e6da62 100644 --- a/gcc/symbol-summary.h +++ b/gcc/symbol-summary.h @@ -31,17 +31,27 @@ public: function_summary_base (symbol_table *symtab CXX_MEM_STAT_INFO): m_symtab (symtab), m_insertion_enabled (true), + m_duplication_enabled (true), m_allocator ("function summary" PASS_MEM_STAT) {} /* Basic implementation of insert operation. */ - virtual void insert (cgraph_node *, T *) {} + virtual void insert (cgraph_node *, T *) + { + /* In most cases, it makes no sense to create summaries without + initializing them. */ + gcc_unreachable (); + } /* Basic implementation of removal operation. */ virtual void remove (cgraph_node *, T *) {} /* Basic implementation of duplication operation. */ - virtual void duplicate (cgraph_node *, cgraph_node *, T *, T *) {} + virtual void duplicate (cgraph_node *, cgraph_node *, T *, T *) + { + /* It makes no sense to not copy anything during duplication. */ + gcc_unreachable (); + } /* Enable insertion hook invocation. */ void enable_insertion_hook () @@ -55,6 +65,18 @@ public: m_insertion_enabled = false; } + /* Enable duplication hook invocation. */ + void enable_duplication_hook () + { + m_duplication_enabled = true; + } + + /* Enable duplication hook invocation. */ + void disable_duplication_hook () + { + m_duplication_enabled = false; + } + protected: /* Allocates new data that are stored within map. */ T* allocate_new () @@ -88,6 +110,8 @@ protected: /* Indicates if insertion hook is enabled. */ bool m_insertion_enabled; + /* Indicates if duplication hook is enabled. */ + bool m_duplication_enabled; private: /* Return true when the summary uses GGC memory for allocation. */ @@ -269,10 +293,13 @@ function_summary::symtab_duplication (cgraph_node *node, cgraph_node *node2, void *data) { function_summary *summary = (function_summary *) (data); - T *v = summary->get (node); + if (summary->m_duplication_enabled) + { + T *v = summary->get (node); - if (v) - summary->duplicate (node, node2, v, summary->get_create (node2)); + if (v) + summary->duplicate (node, node2, v, summary->get_create (node2)); + } } template @@ -468,12 +495,15 @@ fast_function_summary::symtab_duplication (cgraph_node *node, void *data) { fast_function_summary *summary = (fast_function_summary *) (data); - T *v = summary->get (node); - - if (v) + if (summary->m_duplication_enabled) { - T *duplicate = summary->get_create (node2); - summary->duplicate (node, node2, v, duplicate); + T *v = summary->get (node); + + if (v) + { + T *duplicate = summary->get_create (node2); + summary->duplicate (node, node2, v, duplicate); + } } } @@ -536,6 +566,7 @@ public: call_summary_base (symbol_table *symtab CXX_MEM_STAT_INFO): m_symtab (symtab), m_initialize_when_cloning (false), + m_duplication_enabled (true), m_allocator ("call summary" PASS_MEM_STAT) {} @@ -543,7 +574,22 @@ public: virtual void remove (cgraph_edge *, T *) {} /* Basic implementation of duplication operation. */ - virtual void duplicate (cgraph_edge *, cgraph_edge *, T *, T *) {} + virtual void duplicate (cgraph_edge *, cgraph_edge *, T *, T *) + { + gcc_unreachable (); + } + + /* Enable duplication hook invocation. */ + void enable_duplication_hook () + { + m_duplication_enabled = true; + } + + /* Enable duplication hook invocation. */ + void disable_duplication_hook () + { + m_duplication_enabled = false; + } protected: /* Allocates new data that are stored within map. */ @@ -576,6 +622,8 @@ protected: cgraph_2edge_hook_list *m_symtab_duplication_hook; /* Initialize summary for an edge that is cloned. */ bool m_initialize_when_cloning; + /* Indicates if duplication hook is enabled. */ + bool m_duplication_enabled; private: /* Return true when the summary uses GGC memory for allocation. */ @@ -726,16 +774,19 @@ call_summary::symtab_duplication (cgraph_edge *edge1, cgraph_edge *edge2, void *data) { call_summary *summary = (call_summary *) (data); - T *edge1_summary = NULL; + if (summary->m_duplication_enabled) + { + T *edge1_summary = NULL; - if (summary->m_initialize_when_cloning) - edge1_summary = summary->get_create (edge1); - else - edge1_summary = summary->get (edge1); + if (summary->m_initialize_when_cloning) + edge1_summary = summary->get_create (edge1); + else + edge1_summary = summary->get (edge1); - if (edge1_summary) - summary->duplicate (edge1, edge2, edge1_summary, - summary->get_create (edge2)); + if (edge1_summary) + summary->duplicate (edge1, edge2, edge1_summary, + summary->get_create (edge2)); + } } template @@ -892,17 +943,20 @@ fast_call_summary::symtab_duplication (cgraph_edge *edge1, cgraph_edge *edge2, void *data) { fast_call_summary *summary = (fast_call_summary *) (data); - T *edge1_summary = NULL; - - if (summary->m_initialize_when_cloning) - edge1_summary = summary->get_create (edge1); - else - edge1_summary = summary->get (edge1); - - if (edge1_summary) + if (summary->m_duplication_enabled) { - T *duplicate = summary->get_create (edge2); - summary->duplicate (edge1, edge2, edge1_summary, duplicate); + T *edge1_summary = NULL; + + if (summary->m_initialize_when_cloning) + edge1_summary = summary->get_create (edge1); + else + edge1_summary = summary->get (edge1); + + if (edge1_summary) + { + T *duplicate = summary->get_create (edge2); + summary->duplicate (edge1, edge2, edge1_summary, duplicate); + } } } diff --git a/gcc/tree-nested.c b/gcc/tree-nested.c index 788883faa4c..9cb4a08e2d9 100644 --- a/gcc/tree-nested.c +++ b/gcc/tree-nested.c @@ -65,8 +65,11 @@ nested_function_info * nested_function_info::get_create (cgraph_node *node) { if (!nested_function_sum) - nested_function_sum = new function_summary - (symtab); + { + nested_function_sum = new function_summary + (symtab); + nested_function_sum->disable_insertion_hook (); + } return nested_function_sum->get_create (node); } @@ -124,6 +127,9 @@ nested_function_info::release () void maybe_record_nested_function (cgraph_node *node) { + /* All nested functions gets lowered during the construction of symtab. */ + if (symtab->state > CONSTRUCTION) + return; if (DECL_CONTEXT (node->decl) && TREE_CODE (DECL_CONTEXT (node->decl)) == FUNCTION_DECL) {