From patchwork Tue Nov 19 16:52:55 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: David Malcolm X-Patchwork-Id: 292494 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)) (Client did not present a certificate) by ozlabs.org (Postfix) with ESMTPS id C637B2C012B for ; Wed, 20 Nov 2013 03:56:56 +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 :message-id:subject:from:to:cc:date:in-reply-to:references :content-type:mime-version; q=dns; s=default; b=KpAjjgTr9beVb3xk xNIy1zjaOHRJ2nqZsitFGbyWj9q/skt0eTUPL1hMAdW7r2jgy6qXpKZKoRRNm/xJ X5B4h01bfMGJP0sh3BL/pEggztILpaHJByb5v8XLFu2o6pRoMvIgNTIwUQdez6R0 nE8XzbsZ6RC3+PnHM3ZJQ2Gebgg= 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:subject:from:to:cc:date:in-reply-to:references :content-type:mime-version; s=default; bh=+/d8oDvFrh440raye/6SF4 3plhY=; b=tOoJDSTgKiWNVGTLt1GC+GZxT9GQyB0055kWLmpWw+n0TVzfRcTrlk BVir1qjTvnEEnXmyUytpCipqsTeEtX8VvKkHfLvspyrKNR1IazziUJmDcZTMnNiP hBDugLya/GpDwIZsxNQ0ZfJg3IliXHatiIeI1PBTe4jMowq0tbgQY= Received: (qmail 3995 invoked by alias); 19 Nov 2013 16:53:39 -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 3953 invoked by uid 89); 19 Nov 2013 16:53:38 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-0.8 required=5.0 tests=AWL, BAYES_50, RDNS_NONE, SPF_HELO_PASS, SPF_PASS, URIBL_BLOCKED autolearn=no version=3.3.2 X-HELO: mx1.redhat.com Received: from Unknown (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Tue, 19 Nov 2013 16:53:36 +0000 Received: from int-mx12.intmail.prod.int.phx2.redhat.com (int-mx12.intmail.prod.int.phx2.redhat.com [10.5.11.25]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id rAJGrRZm026388 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Tue, 19 Nov 2013 11:53:28 -0500 Received: from [10.3.227.208] (vpn-227-208.phx2.redhat.com [10.3.227.208]) by int-mx12.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id rAJGrQIH032692; Tue, 19 Nov 2013 11:53:27 -0500 Message-ID: <1384879975.11568.117.camel@surprise> Subject: Committed: removal of n_edges macro From: David Malcolm To: Richard Biener Cc: Martin Jambor , Jakub Jelinek , gcc-patches@gcc.gnu.org Date: Tue, 19 Nov 2013 11:52:55 -0500 In-Reply-To: References: <20131113094909.GI27813@tucnak.zalov.cz> <20131113123330.GD10643@virgil.suse> <1384348939.15325.25.camel@surprise> <1384565881.11568.24.camel@surprise> <1384823703.11568.90.camel@surprise> Mime-Version: 1.0 X-IsSubscribed: yes On Tue, 2013-11-19 at 09:49 +0100, Richard Biener wrote: > On Mon, 18 Nov 2013, David Malcolm wrote: > > > On Fri, 2013-11-15 at 20:38 -0500, David Malcolm wrote: > > > On Wed, 2013-11-13 at 14:44 +0100, Richard Biener wrote: > > > > On Wed, 13 Nov 2013, David Malcolm wrote: > > > > > > > > > On Wed, 2013-11-13 at 13:53 +0100, Richard Biener wrote: > > > > > > On Wed, 13 Nov 2013, Martin Jambor wrote: > > > > > > > > > > > > > Hi, > > > > > > > > > > > > > > On Wed, Nov 13, 2013 at 10:49:09AM +0100, Jakub Jelinek wrote: > > > > > > > > Hi! > > > > > > > > > > > > > > > > void f1 (void) {} > > > > > > > > __attribute__((target ("avx"))) void f2 (void) {} > > > > > > > > __attribute__((target ("avx2"))) void f3 (void) {} > > > > > > > > __attribute__((target ("sse3"))) void f4 (void) {} > > > > > > > > __attribute__((target ("ssse3"))) void f5 (void) {} > > > > > > > > __attribute__((target ("sse4"))) void f6 (void) {} > > > > > > > > takes about 3 seconds to compile at -O2, because set_cfun is terribly > > > > > > > > expensive and there are hundreds of such calls. > > > > > > > > The following patch is just a quick change to avoid some of them: > > > > > > > > execute_function_todo starts with: > > > > > > > > unsigned int flags = (size_t)data; > > > > > > > > flags &= ~cfun->last_verified; > > > > > > > > if (!flags) > > > > > > > > return; > > > > > > > > and if flags is initially zero, it does nothing. > > > > > > > > Similarly, execute_function_dump has the whole body surrounded by > > > > > > > > if (dump_file && current_function_decl) > > > > > > > > and thus if dump_file is NULL, there is nothing to do. > > > > > > > > So IMHO in neither case (which happens pretty frequently) we need to > > > > > > > > set_cfun to every function during IPA. > > > > > > > > > > > > > > > > Also, I wonder if we couldn't defer the expensive ira_init, if the info > > > > > > > > computed by it is used only during RTL optimization passes (haven't verified > > > > > > > > it yet), then supposedly we could just remember using some target hook > > > > > > > > what the last state was when we did ira_init last time, and call ira_init > > > > > > > > again at the start of expansion or so if it is different from the > > > > > > > > last time. > > > > > > > > > > > > > > I was wondering whether the expensive parts of set_cfun could only be > > > > > > > run in pass_all_optimizations (and the -Og equivalent) but not when > > > > > > > changing functions in early and IPA passes. > > > > > > > > > > > > Sounds like a hack ;) > > > > > > > > > > > > Better get things working without the cfun/current_function_decl globals. > > > > > > Wasn't there someone replacing all implicit uses with explicit ones > > > > > > for stuff like n_basic_blocks? > > > > > > > > > > I was working on this: > > > > > http://gcc.gnu.org/ml/gcc-patches/2013-06/msg00780.html > > > > > though I switched to other tasks I felt were higher priority; sorry. > > > > > > > > > > Do you still want me to go ahead and commit the series of changes you > > > > > pre-approved there? > > > > > > > > > > i.e. the "n_basic_blocks" macro goes away in favor of: > > > > > n_basic_blocks_for_fn (cfun) > > > > > as a renaming of the existing n_basic_blocks_for_function macro, > > > > > followed up by analogous changes to the other macros. > > > > > > > > > > Or should I repost before committing? > > > > > > > > I'd say create the n_basic_blocks patch and post it, that gives > > > > people a chance to object. If nobody chimes in I approve it > > > > and pre-approve the rest ;) > > > > > > > > Using n_basic_blocks_for_fn (cfun) might feel backwards if > > > > eventually we'd want to C++-ify struct function and make > > > > n_basic_blocks a member function which would make it > > > > cfun->n_basic_blocks () instead. Ok, I think that will get > > > > us into C++ bikeshedding again ;) > > > > > > [I can't face another C vs C++ discussion right now :)] > > > > > > Thanks. Attached is such a patch, eliminating the: > > > n_basic_blocks > > > macro in favor of > > > n_basic_blocks_for_fn (cfun) > > > > > > Successfully bootstrapped on x86_64-unknown-linux-gnu, and successfully > > > compiled stage1 on spu-unknown-elf and s390-linux-gnu (given that those > > > config files are affected). > > > > > > Given the conditional pre-approval above, I'm posting here to give > > > people a change to object - otherwise I'll commit, and followup with the > > > other macros that implicitly use cfun as per the thread linked to above. > > > > Committed to trunk as r204995; I plan to commit followup patches to > > remove the other such macros. > > Thanks! The following removed the "n_edges" macro. I committed it to trunk as r205044 having successfully bootstrapped on x86_64-unknown-linux-gnu. (should I continue to post these patches as I commit them?) From dc32f3cec5bf56cf071a5a7f7b926c2c26d3fe82 Mon Sep 17 00:00:00 2001 From: David Malcolm Date: Mon, 18 Nov 2013 20:23:36 -0500 Subject: [PATCH 1/2] Eliminate n_edges macro gcc/ * basic-block.h (n_edges_for_function): Rename macro to... (n_edges_for_fn): ...this. (n_edges): Eliminate macro as work towards making uses of cfun be explicit. * cfg.c (init_flow): Update for renaming of "n_edges_for_function" to "n_edges_for_fn". * cfg.c (unchecked_make_edge): Remove usage of n_edges macro. (clear_edges): Likewise. (free_edge): Likewise. * cfghooks.c (dump_flow_info): Likewise. * cprop.c (is_too_expensive): Likewise. * df-core.c (df_worklist_dataflow_doublequeue): Likewise. * gcse.c (is_too_expensive): Likewise. (prune_insertions_deletions): Likewise. * mcf.c (create_fixup_graph): Likewise. * sched-rgn.c (haifa_find_rgns): Likewise. * tree-cfg.c (gimple_dump_cfg): Likewise. * var-tracking.c (variable_tracking_main_1): Likewise. --- gcc/basic-block.h | 3 +-- gcc/cfg.c | 8 ++++---- gcc/cfghooks.c | 2 +- gcc/cprop.c | 4 ++-- gcc/df-core.c | 2 +- gcc/gcse.c | 8 ++++---- gcc/mcf.c | 9 ++++++--- gcc/sched-rgn.c | 2 +- gcc/tree-cfg.c | 3 ++- gcc/var-tracking.c | 2 +- 10 files changed, 23 insertions(+), 20 deletions(-) diff --git a/gcc/basic-block.h b/gcc/basic-block.h index d247d4f..38391be 100644 --- a/gcc/basic-block.h +++ b/gcc/basic-block.h @@ -316,7 +316,7 @@ struct GTY(()) control_flow_graph { #define EXIT_BLOCK_PTR_FOR_FUNCTION(FN) ((FN)->cfg->x_exit_block_ptr) #define basic_block_info_for_function(FN) ((FN)->cfg->x_basic_block_info) #define n_basic_blocks_for_fn(FN) ((FN)->cfg->x_n_basic_blocks) -#define n_edges_for_function(FN) ((FN)->cfg->x_n_edges) +#define n_edges_for_fn(FN) ((FN)->cfg->x_n_edges) #define last_basic_block_for_function(FN) ((FN)->cfg->x_last_basic_block) #define label_to_block_map_for_function(FN) ((FN)->cfg->x_label_to_block_map) #define profile_status_for_function(FN) ((FN)->cfg->x_profile_status) @@ -330,7 +330,6 @@ struct GTY(()) control_flow_graph { #define ENTRY_BLOCK_PTR (cfun->cfg->x_entry_block_ptr) #define EXIT_BLOCK_PTR (cfun->cfg->x_exit_block_ptr) #define basic_block_info (cfun->cfg->x_basic_block_info) -#define n_edges (cfun->cfg->x_n_edges) #define last_basic_block (cfun->cfg->x_last_basic_block) #define label_to_block_map (cfun->cfg->x_label_to_block_map) #define profile_status (cfun->cfg->x_profile_status) diff --git a/gcc/cfg.c b/gcc/cfg.c index 10791a7..166ad38 100644 --- a/gcc/cfg.c +++ b/gcc/cfg.c @@ -69,7 +69,7 @@ init_flow (struct function *the_fun) { if (!the_fun->cfg) the_fun->cfg = ggc_alloc_cleared_control_flow_graph (); - n_edges_for_function (the_fun) = 0; + n_edges_for_fn (the_fun) = 0; ENTRY_BLOCK_PTR_FOR_FUNCTION (the_fun) = ggc_alloc_cleared_basic_block_def (); ENTRY_BLOCK_PTR_FOR_FUNCTION (the_fun)->index = ENTRY_BLOCK; @@ -88,7 +88,7 @@ init_flow (struct function *the_fun) static void free_edge (edge e) { - n_edges--; + n_edges_for_fn (cfun)--; ggc_free (e); } @@ -114,7 +114,7 @@ clear_edges (void) vec_safe_truncate (EXIT_BLOCK_PTR->preds, 0); vec_safe_truncate (ENTRY_BLOCK_PTR->succs, 0); - gcc_assert (!n_edges); + gcc_assert (!n_edges_for_fn (cfun)); } /* Allocate memory for basic_block. */ @@ -262,7 +262,7 @@ unchecked_make_edge (basic_block src, basic_block dst, int flags) { edge e; e = ggc_alloc_cleared_edge_def (); - n_edges++; + n_edges_for_fn (cfun)++; e->src = src; e->dest = dst; diff --git a/gcc/cfghooks.c b/gcc/cfghooks.c index 3016c54..20b90bf 100644 --- a/gcc/cfghooks.c +++ b/gcc/cfghooks.c @@ -324,7 +324,7 @@ dump_flow_info (FILE *file, int flags) basic_block bb; fprintf (file, "\n%d basic blocks, %d edges.\n", n_basic_blocks_for_fn (cfun), - n_edges); + n_edges_for_fn (cfun)); FOR_ALL_BB (bb) dump_bb (file, bb, 0, flags); diff --git a/gcc/cprop.c b/gcc/cprop.c index 78cfeba..35a44f2 100644 --- a/gcc/cprop.c +++ b/gcc/cprop.c @@ -1729,12 +1729,12 @@ is_too_expensive (const char *pass) which have a couple switch statements. Rather than simply threshold the number of blocks, uses something with a more graceful degradation. */ - if (n_edges > 20000 + n_basic_blocks_for_fn (cfun) * 4) + if (n_edges_for_fn (cfun) > 20000 + n_basic_blocks_for_fn (cfun) * 4) { warning (OPT_Wdisabled_optimization, "%s: %d basic blocks and %d edges/basic block", pass, n_basic_blocks_for_fn (cfun), - n_edges / n_basic_blocks_for_fn (cfun)); + n_edges_for_fn (cfun) / n_basic_blocks_for_fn (cfun)); return true; } diff --git a/gcc/df-core.c b/gcc/df-core.c index 20d6c4e..37876af 100644 --- a/gcc/df-core.c +++ b/gcc/df-core.c @@ -1097,7 +1097,7 @@ df_worklist_dataflow_doublequeue (struct dataflow *dataflow, fprintf (dump_file, "df_worklist_dataflow_doublequeue:" "n_basic_blocks %d n_edges %d" " count %d (%5.2g)\n", - n_basic_blocks_for_fn (cfun), n_edges, + n_basic_blocks_for_fn (cfun), n_edges_for_fn (cfun), dcount, dcount / (float)n_basic_blocks_for_fn (cfun)); } diff --git a/gcc/gcse.c b/gcc/gcse.c index 5ed99bd..a37ac6b 100644 --- a/gcc/gcse.c +++ b/gcc/gcse.c @@ -1964,7 +1964,7 @@ prune_insertions_deletions (int n_elems) /* Iterate over the edges counting the number of times each expression needs to be inserted. */ - for (i = 0; i < (unsigned) n_edges; i++) + for (i = 0; i < (unsigned) n_edges_for_fn (cfun); i++) { EXECUTE_IF_SET_IN_BITMAP (pre_insert_map[i], 0, j, sbi) insertions[j]++; @@ -1990,7 +1990,7 @@ prune_insertions_deletions (int n_elems) /* Now prune PRE_INSERT_MAP and PRE_DELETE_MAP based on PRUNE_EXPRS. */ EXECUTE_IF_SET_IN_BITMAP (prune_exprs, 0, j, sbi) { - for (i = 0; i < (unsigned) n_edges; i++) + for (i = 0; i < (unsigned) n_edges_for_fn (cfun); i++) bitmap_clear_bit (pre_insert_map[i], j); for (i = 0; i < (unsigned) last_basic_block; i++) @@ -4069,12 +4069,12 @@ is_too_expensive (const char *pass) which have a couple switch statements. Rather than simply threshold the number of blocks, uses something with a more graceful degradation. */ - if (n_edges > 20000 + n_basic_blocks_for_fn (cfun) * 4) + if (n_edges_for_fn (cfun) > 20000 + n_basic_blocks_for_fn (cfun) * 4) { warning (OPT_Wdisabled_optimization, "%s: %d basic blocks and %d edges/basic block", pass, n_basic_blocks_for_fn (cfun), - n_edges / n_basic_blocks_for_fn (cfun)); + n_edges_for_fn (cfun) / n_basic_blocks_for_fn (cfun)); return true; } diff --git a/gcc/mcf.c b/gcc/mcf.c index e0e40d8..45adda3 100644 --- a/gcc/mcf.c +++ b/gcc/mcf.c @@ -472,11 +472,13 @@ create_fixup_graph (fixup_graph_type *fixup_graph) /* Each basic_block will be split into 2 during vertex transformation. */ int fnum_vertices_after_transform = 2 * n_basic_blocks_for_fn (cfun); - int fnum_edges_after_transform = n_edges + n_basic_blocks_for_fn (cfun); + int fnum_edges_after_transform = + n_edges_for_fn (cfun) + n_basic_blocks_for_fn (cfun); /* Count the new SOURCE and EXIT vertices to be added. */ int fmax_num_vertices = - fnum_vertices_after_transform + n_edges + n_basic_blocks_for_fn (cfun) + 2; + (fnum_vertices_after_transform + n_edges_for_fn (cfun) + + n_basic_blocks_for_fn (cfun) + 2); /* In create_fixup_graph: Each basic block and edge can be split into 3 edges. Number of balance edges = n_basic_blocks. So after @@ -486,7 +488,8 @@ create_fixup_graph (fixup_graph_type *fixup_graph) max_edges = 2 * (4 * n_basic_blocks + 3 * n_edges) = 8 * n_basic_blocks + 6 * n_edges < 8 * n_basic_blocks + 8 * n_edges. */ - int fmax_num_edges = 8 * (n_basic_blocks_for_fn (cfun) + n_edges); + int fmax_num_edges = 8 * (n_basic_blocks_for_fn (cfun) + + n_edges_for_fn (cfun)); /* Initial num of vertices in the fixup graph. */ fixup_graph->num_vertices = n_basic_blocks_for_fn (cfun); diff --git a/gcc/sched-rgn.c b/gcc/sched-rgn.c index 20c29c5..87042dd 100644 --- a/gcc/sched-rgn.c +++ b/gcc/sched-rgn.c @@ -643,7 +643,7 @@ haifa_find_rgns (void) /* Allocate and initialize variables for the first traversal. */ max_hdr = XNEWVEC (int, last_basic_block); dfs_nr = XCNEWVEC (int, last_basic_block); - stack = XNEWVEC (edge_iterator, n_edges); + stack = XNEWVEC (edge_iterator, n_edges_for_fn (cfun)); inner = sbitmap_alloc (last_basic_block); bitmap_ones (inner); diff --git a/gcc/tree-cfg.c b/gcc/tree-cfg.c index c30b113..d2af39e 100644 --- a/gcc/tree-cfg.c +++ b/gcc/tree-cfg.c @@ -2106,7 +2106,8 @@ gimple_dump_cfg (FILE *file, int flags) { dump_function_header (file, current_function_decl, flags); fprintf (file, ";; \n%d basic blocks, %d edges, last basic block %d.\n\n", - n_basic_blocks_for_fn (cfun), n_edges, last_basic_block); + n_basic_blocks_for_fn (cfun), n_edges_for_fn (cfun), + last_basic_block); brief_dump_cfg (file, flags | TDF_COMMENT); fprintf (file, "\n"); diff --git a/gcc/var-tracking.c b/gcc/var-tracking.c index a569d46..cfda63a 100644 --- a/gcc/var-tracking.c +++ b/gcc/var-tracking.c @@ -10161,7 +10161,7 @@ variable_tracking_main_1 (void) } if (n_basic_blocks_for_fn (cfun) > 500 && - n_edges / n_basic_blocks_for_fn (cfun) >= 20) + n_edges_for_fn (cfun) / n_basic_blocks_for_fn (cfun) >= 20) { vt_debug_insns_local (true); return 0; -- 1.7.11.7