From patchwork Mon Dec 8 17:01:53 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Martin_Li=C5=A1ka?= X-Patchwork-Id: 418759 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 3636C14019D for ; Tue, 9 Dec 2014 04:02:07 +1100 (AEDT) DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender :message-id:date:from:mime-version:to:subject:references :in-reply-to:content-type; q=dns; s=default; b=gcWju+wRuNH7fBhCO ilWXZEQsd/iNqHTTue90Hc5kfTkGqhBa0E0FHZAhuEidXRXgV7MwzsfVZn9WjuuC miEkUsgjjqst2Sdt57rT7+x8F8V+PMrX2QeU4lyXyOPfCV94EoUzzJ6Iky+SAWBY zUdWwVC5rUl335JO5hMMYTUTuQ= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender :message-id:date:from:mime-version:to:subject:references :in-reply-to:content-type; s=default; bh=5M35KUGkeK9R3TlRlNSv9I2 mlsA=; b=ZijHTJ/NuuFTdZxTht+uD8fJ0q3VlEvf5JZYxOKQJQVVTlN0GTXtfRR w339F3y8K8DVE2eL1qFkbtJs4jS3mrvXm2p4wkjsUHlHl+TCLTbkKlE+6Uu2mOMJ ALEug46zgciBrQhZ1u1IRK1tDW+eyO+ITlmRdI+fcLiYfpbW0duk= Received: (qmail 13902 invoked by alias); 8 Dec 2014 17:01:59 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org Received: (qmail 13889 invoked by uid 89); 8 Dec 2014 17:01:59 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-0.3 required=5.0 tests=AWL, BAYES_00, KAM_STOCKGEN autolearn=no version=3.3.2 X-HELO: mx2.suse.de Received: from cantor2.suse.de (HELO mx2.suse.de) (195.135.220.15) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (CAMELLIA256-SHA encrypted) ESMTPS; Mon, 08 Dec 2014 17:01:57 +0000 Received: from relay2.suse.de (charybdis-ext.suse.de [195.135.220.254]) by mx2.suse.de (Postfix) with ESMTP id C86FFAD7D for ; Mon, 8 Dec 2014 17:01:53 +0000 (UTC) Message-ID: <5485D981.10203@suse.cz> Date: Mon, 08 Dec 2014 18:01:53 +0100 From: =?windows-1252?Q?Martin_Li=9Aka?= User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.2.0 MIME-Version: 1.0 To: gcc-patches@gcc.gnu.org Subject: Re: [PATCH 2/4] New data structure for cgraph_summary introduced. References: <8d6246a176f0270dee1f5b190e8ecbddebee342f.1415888515.git.mliska@suse.cz> <20141113155041.GF11013@kam.mff.cuni.cz> <54660BF7.1020506@suse.cz> <54661E6C.3070600@suse.cz> In-Reply-To: <54661E6C.3070600@suse.cz> X-IsSubscribed: yes On 11/14/2014 04:23 PM, Martin Liška wrote: > On 11/14/2014 03:04 PM, Martin Liška wrote: >> On 11/13/2014 04:50 PM, Jan Hubicka wrote: >>>> gcc/ChangeLog: >>>> >>>> 2014-11-12 Martin Liska >>>> >>>> * Makefile.in: New object file is added. >>>> * cgraph.h (symbol_table::allocate_cgraph_symbol): Summary UID >>>> is filled up. >>>> * cgraph_summary.c: New file. >>>> * cgraph_summary.h: New file. >>> >>> Since I am trying to get rid of the cgraph prefixes for symbols (keep it for >>> the graph only) and the summaries can be annotated to variables too. Even if it >>> not necessarily supported by your current implementation, lets keep API >>> prepared for it. So I would call it symtab-summary.* for source files and >>> symtab_summary for base type (probably function_summary for annotating >>> functions/cgraph_edge_summary for annotating edges?) >> >> Hello. >> >> I followed your remarks, new class is called function_summary and is located >> in symbol-summary.h. >>> >>> >>>> diff --git a/gcc/cgraph.h b/gcc/cgraph.h >>>> index e2becb9..588b6d5 100644 >>>> --- a/gcc/cgraph.h >>>> +++ b/gcc/cgraph.h >>>> @@ -1225,6 +1225,8 @@ public: >>>> int count_materialization_scale; >>>> /* Unique id of the node. */ >>>> int uid; >>>> + /* Summary unique id of the node. */ >>>> + int summary_uid; >>> >>> What makes summary_uid better than uid? >> >> Because cgraph_node::uid is not a unique ID, it's recycled. As I can see, >> there are two remaining usages of the fact that cgraph::uid are quite consecutive: >> >> a) node_growth_cache vector is resized according to cgraph_max_uid >> b) lto-partition.c: lto_balanced_map >> >> If we change ipa-related stuff to annotations and lto_balanced_map with be rewritten, >> we can finally unify uid and summary_uid. As Martin correctly pointed out, we should >> unify cgraph_node dumps, we combine uid and order. >> >>> >>>> diff --git a/gcc/cgraph_summary.c b/gcc/cgraph_summary.c >>>> new file mode 100644 >>>> index 0000000..9af1d7e >>>> --- /dev/null >>>> +++ b/gcc/cgraph_summary.c >>> >>> And why do we need this file? It will need license header if really needed. >> >> Sure, the file can be removed. >> >> Martin >> >>> >>> The implementation seems sane - I will check the actual uses :) >>> Please send the updated patch though. >>> >>> Honza >>> >> > > Hello. > > There's v3 of the patch. > > Martin Hello. Version 4 of the patch. I have a conversation with Trevor and I hope it can be acceptable to trunk? Patch set can bootstrap on x86_64-linux-pc and no regression has been seen. Thanks, Martin From df978f4a338869b56dc558c70f6c8f3c884b9fe1 Mon Sep 17 00:00:00 2001 From: mliska Date: Fri, 5 Dec 2014 15:59:04 +0100 Subject: [PATCH 1/3] New symbol_summary class introduced. gcc/ChangeLog: 2014-12-08 Martin Liska * cgraph.h (symbol_table::allocate_cgraph_symbol): Summary UID is filled up. * symbol-summary.h: New file. * gengtype.c (open_base_files): Add symbol-summary.h. * toplev.c (general_init): Call constructor of symbol_table. --- gcc/cgraph.h | 8 ++ gcc/gengtype.c | 4 +- gcc/symbol-summary.h | 281 +++++++++++++++++++++++++++++++++++++++++++++++++++ gcc/toplev.c | 3 +- 4 files changed, 293 insertions(+), 3 deletions(-) create mode 100644 gcc/symbol-summary.h diff --git a/gcc/cgraph.h b/gcc/cgraph.h index a5c5f56..1664bd7 100644 --- a/gcc/cgraph.h +++ b/gcc/cgraph.h @@ -1237,6 +1237,8 @@ public: int count_materialization_scale; /* Unique id of the node. */ int uid; + /* Summary unique id of the node. */ + int summary_uid; /* ID assigned by the profiling. */ unsigned int profile_id; /* Time profiler: first run of function. */ @@ -1810,6 +1812,10 @@ public: friend class cgraph_node; friend class cgraph_edge; + symbol_table (): cgraph_max_summary_uid (1) + { + } + /* Initialize callgraph dump file. */ void initialize (void); @@ -2006,6 +2012,7 @@ public: int cgraph_count; int cgraph_max_uid; + int cgraph_max_summary_uid; int edges_count; int edges_max_uid; @@ -2334,6 +2341,7 @@ symbol_table::allocate_cgraph_symbol (void) node->uid = cgraph_max_uid++; } + node->summary_uid = cgraph_max_summary_uid++; return node; } diff --git a/gcc/gengtype.c b/gcc/gengtype.c index 2dc857e..276f543 100644 --- a/gcc/gengtype.c +++ b/gcc/gengtype.c @@ -1842,8 +1842,8 @@ open_base_files (void) "tree-ssa-loop-niter.h", "tree-into-ssa.h", "tree-dfa.h", "tree-ssa.h", "reload.h", "cpp-id-data.h", "tree-chrec.h", "except.h", "output.h", "cfgloop.h", "target.h", "lto-streamer.h", - "target-globals.h", "ipa-ref.h", "cgraph.h", "ipa-prop.h", - "ipa-inline.h", "dwarf2out.h", "omp-low.h", NULL + "target-globals.h", "ipa-ref.h", "cgraph.h", "symbol-summary.h", + "ipa-prop.h", "ipa-inline.h", "dwarf2out.h", "omp-low.h", NULL }; const char *const *ifp; outf_p gtype_desc_c; diff --git a/gcc/symbol-summary.h b/gcc/symbol-summary.h new file mode 100644 index 0000000..95270e5 --- /dev/null +++ b/gcc/symbol-summary.h @@ -0,0 +1,281 @@ +/* Callgraph summary data structure. + Copyright (C) 2014 Free Software Foundation, Inc. + Contributed by Martin Liska + +This file is part of GCC. + +GCC is free software; you can redistribute it and/or modify it under +the terms of the GNU General Public License as published by the Free +Software Foundation; either version 3, or (at your option) any later +version. + +GCC is distributed in the hope that it will be useful, but WITHOUT ANY +WARRANTY; without even the implied warranty of MERCHANTABILITY or +FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License +for more details. + +You should have received a copy of the GNU General Public License +along with GCC; see the file COPYING3. If not see +. */ + +#ifndef GCC_SYMBOL_SUMMARY_H +#define GCC_SYMBOL_SUMMARY_H + +/* We want to pass just pointer types as argument for function_summary + template class. */ + +template +class function_summary +{ +private: + function_summary(); +}; + +template +class GTY((user)) function_summary +{ +public: + /* Default construction takes SYMTAB as an argument. */ + function_summary (symbol_table *symtab, bool ggc = false): m_ggc (ggc), + m_map (13, ggc), m_insertion_enabled (true), m_symtab (symtab) + { + cgraph_node *node; + +#ifdef ENABLE_CHECKING + FOR_EACH_FUNCTION (node) + { + gcc_checking_assert (node->summary_uid > 0); + } +#endif + + m_symtab_insertion_hook = + symtab->add_cgraph_insertion_hook + (function_summary::symtab_insertion, this); + + m_symtab_removal_hook = + symtab->add_cgraph_removal_hook + (function_summary::symtab_removal, this); + m_symtab_duplication_hook = + symtab->add_cgraph_duplication_hook + (function_summary::symtab_duplication, this); + } + + /* Destructor. */ + virtual ~function_summary () + { + release (); + } + + /* Destruction method that can be called for GGT purpose. */ + void release () + { + if (m_symtab_insertion_hook) + m_symtab->remove_cgraph_insertion_hook (m_symtab_insertion_hook); + + if (m_symtab_removal_hook) + m_symtab->remove_cgraph_removal_hook (m_symtab_removal_hook); + + if (m_symtab_duplication_hook) + m_symtab->remove_cgraph_duplication_hook (m_symtab_duplication_hook); + + m_symtab_insertion_hook = NULL; + m_symtab_removal_hook = NULL; + m_symtab_duplication_hook = NULL; + } + + /* Traverses all summarys with a function F called with + ARG as argument. */ + template + void traverse (Arg a) const + { + m_map.traverse (a); + } + + /* Basic implementation of insert operation. */ + virtual void insert (cgraph_node *, T *) {} + + /* 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 *) {} + + /* Allocates new data that are stored within map. */ + T* allocate_new () + { + return m_ggc ? new (ggc_alloc ()) T() : new T () ; + } + + /* Getter for summary callgraph node pointer. */ + T* get (cgraph_node *node) + { + return get (node->summary_uid); + } + + /* Return number of elements handled by data structure. */ + size_t elements () + { + return m_map.elements (); + } + + /* Enable insertion hook invocation. */ + void enable_insertion_hook () + { + m_insertion_enabled = true; + } + + /* Enable insertion hook invocation. */ + void disable_insertion_hook () + { + m_insertion_enabled = false; + } + + /* Symbol insertion hook that is registered to symbol table. */ + static void symtab_insertion (cgraph_node *node, void *data) + { + function_summary *summary = (function_summary *) (data); + + if (summary->m_insertion_enabled) + summary->insert (node, summary->get (node)); + } + + /* Symbol removal hook that is registered to symbol table. */ + static void symtab_removal (cgraph_node *node, void *data) + { + gcc_checking_assert (node->summary_uid); + function_summary *summary = (function_summary *) (data); + + int summary_uid = node->summary_uid; + T **v = summary->m_map.get (summary_uid); + + if (v) + { + summary->remove (node, *v); + + if (!summary->m_ggc) + delete (*v); + + summary->m_map.remove (summary_uid); + } + } + + /* Symbol duplication hook that is registered to symbol table. */ + static void symtab_duplication (cgraph_node *node, cgraph_node *node2, + void *data) + { + function_summary *summary = (function_summary *) (data); + T **v = summary->m_map.get (node->summary_uid); + + gcc_checking_assert (node2->summary_uid > 0); + + if (v) + { + /* This load is necessary, because we insert a new value! */ + T *data = *v; + T *duplicate = summary->allocate_new (); + summary->m_map.put (node2->summary_uid, duplicate); + summary->duplicate (node, node2, data, duplicate); + } + } + +protected: + /* Indication if we use ggc summary. */ + bool m_ggc; + +private: + struct summary_hashmap_traits: default_hashmap_traits + { + static const int deleted_value = -1; + static const int empty_value = 0; + + static hashval_t + hash (const int v) + { + return (hashval_t)v; + } + + template + static bool + is_deleted (Type &e) + { + return e.m_key == deleted_value; + } + + template + static bool + is_empty (Type &e) + { + return e.m_key == empty_value; + } + + template + static void + mark_deleted (Type &e) + { + e.m_key = deleted_value; + } + + template + static void + mark_empty (Type &e) + { + e.m_key = empty_value; + } + }; + + /* Getter for summary callgraph ID. */ + T* get (int uid) + { + bool existed; + T **v = &m_map.get_or_insert (uid, &existed); + if (!existed) + *v = allocate_new (); + + return *v; + } + + /* Main summary store, where summary ID is used as key. */ + hash_map m_map; + /* Internal summary insertion hook pointer. */ + cgraph_node_hook_list *m_symtab_insertion_hook; + /* Internal summary removal hook pointer. */ + cgraph_node_hook_list *m_symtab_removal_hook; + /* Internal summary duplication hook pointer. */ + cgraph_2node_hook_list *m_symtab_duplication_hook; + /* Indicates if insertion hook is enabled. */ + bool m_insertion_enabled; + /* Symbol table the summary is registered to. */ + symbol_table *m_symtab; + + template friend void gt_ggc_mx (function_summary * const &); + template friend void gt_pch_nx (function_summary * const &); + template friend void gt_pch_nx (function_summary * const &, + gt_pointer_operator, void *); +}; + +template +void +gt_ggc_mx(function_summary* const &summary) +{ + gcc_checking_assert (summary->m_ggc); + gt_ggc_mx (&summary->m_map); +} + +template +void +gt_pch_nx(function_summary* const &summary) +{ + gcc_checking_assert (summary->m_ggc); + gt_pch_nx (&summary->m_map); +} + +template +void +gt_pch_nx(function_summary* const& summary, gt_pointer_operator op, + void *cookie) +{ + gcc_checking_assert (summary->m_ggc); + gt_pch_nx (&summary->m_map, op, cookie); +} + +#endif /* GCC_SYMBOL_SUMMARY_H */ diff --git a/gcc/toplev.c b/gcc/toplev.c index 6e6adfa..6ce3f7b 100644 --- a/gcc/toplev.c +++ b/gcc/toplev.c @@ -93,6 +93,7 @@ along with GCC; see the file COPYING3. If not see #include "dwarf2out.h" #include "bitmap.h" #include "ipa-reference.h" +#include "symbol-summary.h" #include "ipa-prop.h" #include "gcse.h" #include "insn-codes.h" @@ -1213,7 +1214,7 @@ general_init (const char *argv0) /* Create the singleton holder for global state. Doing so also creates the pass manager and with it the passes. */ g = new gcc::context (); - symtab = ggc_cleared_alloc (); + symtab = new (ggc_cleared_alloc ()) symbol_table (); statistics_early_init (); finish_params (); -- 2.1.2