From patchwork Thu Mar 27 16:02:14 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Dehao Chen X-Patchwork-Id: 334389 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 725CF14008D for ; Fri, 28 Mar 2014 03:02:29 +1100 (EST) DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender :mime-version:in-reply-to:references:date:message-id:subject :from:to:cc:content-type; q=dns; s=default; b=fi3WbckpUQRSX9937L i7XXnqYX4oq8lpOgcMkOBqi0XbqHV+AaByP32nQn7KfL1FAzPbuv6Su1rT6YbhI3 5dt2TH5Mb48ayf68ha1+MjoKA4OoU/U2AN5wScJ/O+G73z95WLeA3JflNAUReFKA fyVmYoOMSuBQXs1KupAvOIVx0= 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 :mime-version:in-reply-to:references:date:message-id:subject :from:to:cc:content-type; s=default; bh=yUtzjss4MACvNUMNzaQL9duU yi0=; b=v7EvveJCFklSPLl2NTqhiKMxfjMpw6wXn0oCfVhrwspGksyBf7GCwcqY 5KQ5PGlFwVm35MX5ASsSRz8vE71F1lxMH4wkE4ZwPXDLe8PaAv16NGyT3Btwir2I ue2w3ug1v/igcaDRi3UyMF6XMh52mSKKLwTZuMj1wHePjI04wBk= Received: (qmail 22521 invoked by alias); 27 Mar 2014 16:02:22 -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 22510 invoked by uid 89); 27 Mar 2014 16:02:21 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-3.2 required=5.0 tests=AWL, BAYES_00, RCVD_IN_DNSWL_LOW, RP_MATCHES_RCVD, SPF_PASS autolearn=ham version=3.3.2 X-HELO: mail-ie0-f170.google.com Received: from mail-ie0-f170.google.com (HELO mail-ie0-f170.google.com) (209.85.223.170) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-SHA encrypted) ESMTPS; Thu, 27 Mar 2014 16:02:16 +0000 Received: by mail-ie0-f170.google.com with SMTP id rd18so3658394iec.29 for ; Thu, 27 Mar 2014 09:02:14 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:in-reply-to:references:date :message-id:subject:from:to:cc:content-type; bh=lKzAX5iKFSsdH4+sJinlX0EtYj7Mfxv/O44U9XQTGZs=; b=MKR5hrWkZ/w+bnBrPqGDwnbG+u6lHTe4lV9O2EdAKYbRTUzCTZ9g65R03t3dZTZlZP aOOpSVD2IiNA7B+Nr5syNabsK2zXPc2hMTkp8ghROO3UeKEC/k7M/hjeucTExn+A6iZf ktZeDrF3LWJ+swyYX+RKz8TYIrAtBKK9bvGGPc1v5zkmgqGRkWkhg2WsWIS2KcLxRUZq QiDPjJmjEeDODDOfZAmpB3+CziCFDImWtYUA2J7AG2ATFFfSgoK3VPBOmoK7jvvpmv55 uJSoWFlNLPftD7udqHW3TFgY6GfRQdCVk2DpHOsqMbTY9JzuCV/Ldh9JhueUzhpEljcb rPkA== X-Gm-Message-State: ALoCoQmftWhy/hXgzqfT0ro9xPuYYiStn0IHFMXqxX33qSB8NttUyPpC2c84SdS4nUgM56X8SBF92wK2E5EFpPIQ2j27EJ/gWuxAQMfF7+WO7Adl2rlBP0VtOBnQv0PGHFOIaLHI+70pR88VUApeKplH4dl3P976d7xvljQrBk72AKlCbIpVdQWF5RnAJ39k5Z80FqIVtS61Kq2mvgzrbbxm+Jc1PJ/FtQ== MIME-Version: 1.0 X-Received: by 10.42.64.17 with SMTP id e17mr2791677ici.26.1395936134787; Thu, 27 Mar 2014 09:02:14 -0700 (PDT) Received: by 10.64.18.207 with HTTP; Thu, 27 Mar 2014 09:02:14 -0700 (PDT) In-Reply-To: References: Date: Thu, 27 Mar 2014 09:02:14 -0700 Message-ID: Subject: Re: [GOOGLE] Refactor the LIPO fixup From: Dehao Chen To: Xinliang David Li Cc: GCC Patches , Rong Xu X-IsSubscribed: yes On Wed, Mar 26, 2014 at 4:05 PM, Xinliang David Li wrote: > is cgraph_init_gid_map called after linking? Oh, forgot that part. It's interesting that the test can pass without another cgraph_init_gid_map call. Patch updated. Retested and the performance is OK. Dehao > > David > > On Wed, Mar 26, 2014 at 3:54 PM, Dehao Chen wrote: >> Patch updated, passed performance tests. >> >> Dehao >> >> On Tue, Mar 25, 2014 at 4:03 PM, Xinliang David Li wrote: >>> Add comment to the new function. init_node_map is better invoked after >>> the link step to avoid creating entries with for dead nodes. >>> >>> Ok if large perf testing is fine. >>> >>> David >>> >>> On Tue, Mar 25, 2014 at 3:38 PM, Dehao Chen wrote: >>>> This patch refactors LIPO fixup related code to move it into a >>>> standalone function. This makes sure that >>>> symtab_remove_unreachable_nodes is called right after the fixup so >>>> that there is not dangling cgraph nodes any time. >>>> >>>> Bootstrapped and regression test on-going. >>>> >>>> OK for google-4_8? >>>> >>>> Thanks, >>>> Dehao Index: gcc/cgraphbuild.c =================================================================== --- gcc/cgraphbuild.c (revision 208869) +++ gcc/cgraphbuild.c (working copy) @@ -244,9 +244,6 @@ add_fake_indirect_call_edges (struct cgraph_node * if (!L_IPO_COMP_MODE) return; - if (cgraph_pre_profiling_inlining_done) - return; - ic_counts = get_coverage_counts_no_warn (DECL_STRUCT_FUNCTION (node->symbol.decl), GCOV_COUNTER_ICALL_TOPNV, &n_counts); @@ -599,7 +596,7 @@ record_references_in_initializer (tree decl, bool needs to be set to the resolved node so that ipa-inline sees the definitions. */ #include "gimple-pretty-print.h" -void +static void lipo_fixup_cgraph_edge_call_target (gimple stmt) { tree decl; @@ -625,6 +622,58 @@ lipo_fixup_cgraph_edge_call_target (gimple stmt) } } +/* Link the cgraph nodes, varpool nodes and fixup the call target to + the correct decl. Remove dead functions. */ + + +void +lipo_link_and_fixup () +{ + struct cgraph_node *node; + + cgraph_pre_profiling_inlining_done = true; + cgraph_process_module_scope_statics (); + /* Now perform link to allow cross module inlining. */ + cgraph_do_link (); + varpool_do_link (); + cgraph_unify_type_alias_sets (); + cgraph_init_gid_map (); + + FOR_EACH_DEFINED_FUNCTION (node) + { + if (!gimple_has_body_p (node->symbol.decl)) + continue; + + /* Don't profile functions produced for builtin stuff. */ + if (DECL_SOURCE_LOCATION (node->symbol.decl) == BUILTINS_LOCATION) + continue; + + push_cfun (DECL_STRUCT_FUNCTION (node->symbol.decl)); + + if (L_IPO_COMP_MODE) + { + basic_block bb; + FOR_EACH_BB (bb) + { + gimple_stmt_iterator gsi; + for (gsi = gsi_start_bb (bb); !gsi_end_p (gsi); gsi_next (&gsi)) + { + gimple stmt = gsi_stmt (gsi); + if (is_gimple_call (stmt)) + lipo_fixup_cgraph_edge_call_target (stmt); + } + } + update_ssa (TODO_update_ssa); + } + rebuild_cgraph_edges (); + pop_cfun (); + } + + cgraph_add_fake_indirect_call_edges (); + symtab_remove_unreachable_nodes (true, dump_file); +} + + /* Rebuild cgraph edges for current function node. This needs to be run after passes that don't update the cgraph. */ @@ -677,7 +726,8 @@ rebuild_cgraph_edges (void) mark_load, mark_store, mark_address); } - add_fake_indirect_call_edges (node); + if (!cgraph_pre_profiling_inlining_done) + add_fake_indirect_call_edges (node); record_eh_tables (node, cfun); gcc_assert (!node->global.inlined_to); Index: gcc/l-ipo.h =================================================================== --- gcc/l-ipo.h (revision 208869) +++ gcc/l-ipo.h (working copy) @@ -60,7 +60,7 @@ void add_decl_to_current_module_scope (tree decl, int lipo_cmp_type (tree t1, tree t2); tree get_type_or_decl_name (tree); int equivalent_struct_types_for_tbaa (const_tree t1, const_tree t2); -void lipo_fixup_cgraph_edge_call_target (gimple); +void lipo_link_and_fixup (void); extern void copy_defined_module_set (tree, tree); extern bool is_parsing_done_p (void); extern const char* get_module_name (unsigned int); Index: gcc/tree-profile.c =================================================================== --- gcc/tree-profile.c (revision 208869) +++ gcc/tree-profile.c (working copy) @@ -1118,19 +1118,12 @@ tree_profiling (void) /* This is a small-ipa pass that gets called only once, from cgraphunit.c:ipa_passes(). */ gcc_assert (cgraph_state == CGRAPH_STATE_IPA_SSA); - /* After value profile transformation, artificial edges (that keep function body from being deleted) won't be needed. */ + if (L_IPO_COMP_MODE) + lipo_link_and_fixup (); + init_node_map (); - cgraph_pre_profiling_inlining_done = true; - cgraph_process_module_scope_statics (); - /* Now perform link to allow cross module inlining. */ - cgraph_do_link (); - varpool_do_link (); - cgraph_unify_type_alias_sets (); - - init_node_map(); - FOR_EACH_DEFINED_FUNCTION (node) { if (!gimple_has_body_p (node->symbol.decl)) @@ -1142,23 +1135,6 @@ tree_profiling (void) push_cfun (DECL_STRUCT_FUNCTION (node->symbol.decl)); - if (L_IPO_COMP_MODE) - { - basic_block bb; - FOR_EACH_BB (bb) - { - gimple_stmt_iterator gsi; - for (gsi = gsi_start_bb (bb); !gsi_end_p (gsi); gsi_next (&gsi)) - { - gimple stmt = gsi_stmt (gsi); - if (is_gimple_call (stmt)) - lipo_fixup_cgraph_edge_call_target (stmt); - } - } - update_ssa (TODO_update_ssa); - } - - /* Local pure-const may imply need to fixup the cfg. */ if (execute_fixup_cfg () & TODO_cleanup_cfg) cleanup_tree_cfg (); Index: gcc/value-prof.c =================================================================== --- gcc/value-prof.c (revision 208869) +++ gcc/value-prof.c (working copy) @@ -1255,11 +1255,9 @@ init_gid_map (void) { struct cgraph_node *n; - gcc_assert (!gid_map); + if (!gid_map) + gid_map = htab_create (10, htab_gid_hash, htab_gid_eq, htab_gid_del); - gid_map - = htab_create (10, htab_gid_hash, htab_gid_eq, htab_gid_del); - FOR_EACH_FUNCTION (n) { func_gid_entry_t ent, *entp; @@ -1286,6 +1284,15 @@ init_gid_map (void) entp->node = n; entp->gid = ent.gid; } + else if (cgraph_pre_profiling_inlining_done) + { + (*slot)->node = cgraph_lipo_get_resolved_node (n->symbol.decl); + (*slot)->gid = ent.gid; + } + else + { + gcc_assert ((*slot)->gid == ent.gid && (*slot)->node == n); + } } } Index: gcc/auto-profile.c =================================================================== --- gcc/auto-profile.c (revision 208869) +++ gcc/auto-profile.c (working copy) @@ -1533,16 +1533,11 @@ auto_profile (void) if (cgraph_state == CGRAPH_STATE_FINISHED) return 0; - init_node_map (); profile_info = autofdo::afdo_profile_info; + if (L_IPO_COMP_MODE) + lipo_link_and_fixup (); + init_node_map (); - cgraph_pre_profiling_inlining_done = true; - cgraph_process_module_scope_statics (); - /* Now perform link to allow cross module inlining. */ - cgraph_do_link (); - varpool_do_link (); - cgraph_unify_type_alias_sets (); - FOR_EACH_FUNCTION (node) { if (!gimple_has_body_p (node->symbol.decl)) @@ -1554,35 +1549,6 @@ auto_profile (void) push_cfun (DECL_STRUCT_FUNCTION (node->symbol.decl)); - if (L_IPO_COMP_MODE) - { - basic_block bb; - FOR_EACH_BB (bb) - { - gimple_stmt_iterator gsi; - for (gsi = gsi_start_bb (bb); !gsi_end_p (gsi); gsi_next (&gsi)) - { - gimple stmt = gsi_stmt (gsi); - if (is_gimple_call (stmt)) - lipo_fixup_cgraph_edge_call_target (stmt); - } - } - } - rebuild_cgraph_edges (); - pop_cfun (); - } - - FOR_EACH_FUNCTION (node) - { - if (!gimple_has_body_p (node->symbol.decl)) - continue; - - /* Don't profile functions produced for builtin stuff. */ - if (DECL_SOURCE_LOCATION (node->symbol.decl) == BUILTINS_LOCATION) - continue; - - push_cfun (DECL_STRUCT_FUNCTION (node->symbol.decl)); - /* First do indirect call promotion and early inline to make the IR match the profiled binary before actual annotation.