From patchwork Wed Jun 23 10:15:45 2010 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Jan Hubicka X-Patchwork-Id: 56658 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]) by ozlabs.org (Postfix) with SMTP id A0A501024BF for ; Wed, 23 Jun 2010 20:15:57 +1000 (EST) Received: (qmail 4279 invoked by alias); 23 Jun 2010 10:15:55 -0000 Received: (qmail 4270 invoked by uid 22791); 23 Jun 2010 10:15:54 -0000 X-SWARE-Spam-Status: No, hits=-1.8 required=5.0 tests=AWL, BAYES_00, T_RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from nikam-dmz.ms.mff.cuni.cz (HELO nikam.ms.mff.cuni.cz) (195.113.20.16) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Wed, 23 Jun 2010 10:15:48 +0000 Received: by nikam.ms.mff.cuni.cz (Postfix, from userid 16202) id 9632B9AC851; Wed, 23 Jun 2010 12:15:45 +0200 (CEST) Date: Wed, 23 Jun 2010 12:15:45 +0200 From: Jan Hubicka To: Steven Bosscher Cc: Jan Hubicka , gcc-patches@gcc.gnu.org Subject: Re: Make dominated_by_p and get_immediate_dominator inline Message-ID: <20100623101545.GA9710@kam.mff.cuni.cz> References: <20100622183636.GC23996@kam.mff.cuni.cz> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.18 (2008-05-17) 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 > On Tue, Jun 22, 2010 at 8:36 PM, Jan Hubicka wrote: > > Hi, > > this is change I forgot in my tree for a while.  get_immediate_dominator and > > dominated_by_p are one of most frequently called functions and they are good > > candidates for inlining. Dominated_by_p has fallback path calling et_forest > > code.  Most of passes don't use dynamic dominance info and thus we never get > > there.  I was thinking about making specific version for non-dynamic dominators, > > but since the most common calls from dominated_by_p comes from cfg.c > > (dominance frontiers computation) that is sort of generic, it would > > require more bookkeping of when info is dynamic and when not. > > Hopefully the cold path is not going to cause too much of trouble. > > > > The patch needs to include et-forest.h in basic-block.h, but I do not see > > much way around. > > Do not make this inline? > > I think making all these small functions inline makes things only more > entangled. Now you include et-forest.h in every place that includes > basic-block.h. I see no point in continuing my little project of > removing #include's if you plan to add more of these. > > I would much rather *remove* inline functions and let WHOPR do its job! I am defnitly trying to push things towards making WHOPR the official way to build GCC binary. Hopefully we will get there for next release. Even with WHOPR we won't inline dominated_by_p, at least as long as we build with -O2 because inlining it increases cc1 binary size (by 14kb). So at least we would have to declare it inline in dominance.c Averaging 5 runs of each I got 9m4s withtout the patch and 9m2s with the patch. dominated_by_p accounts to about 0.4% of runtime according to oprofile. It seems that we do too many queries using et_splay that acounts 0.56% of compilation time. I guess it is because we do not recompute and keep dominance tree in NO_FAST_QUERY mode for a while. Will check. Well, if we agree that we do not care about this relatively little amount of perfomrance for non-WHOPR build, I guess following patch would be less intrusive ;) Bootstrapping/regtesting x86_64-linux, OK? Honza * dominance.c (get_immediate_dominator, set_immediate_dominator, dom_convert_dir_to_idx): Make inline; use checking assert. Index: dominance.c =================================================================== --- dominance.c (revision 161199) +++ dominance.c (working copy) @@ -188,10 +188,10 @@ init_dom_info (struct dom_info *di, enum to be modified, obviously, if additional values are added to cdi_direction. */ -static unsigned int +static inline unsigned int dom_convert_dir_to_idx (enum cdi_direction dir) { - gcc_assert (dir == CDI_DOMINATORS || dir == CDI_POST_DOMINATORS); + gcc_checking_assert (dir == CDI_DOMINATORS || dir == CDI_POST_DOMINATORS); return dir - 1; } @@ -696,13 +696,13 @@ free_dominance_info (enum cdi_direction } /* Return the immediate dominator of basic block BB. */ -basic_block +inline basic_block get_immediate_dominator (enum cdi_direction dir, basic_block bb) { unsigned int dir_index = dom_convert_dir_to_idx (dir); struct et_node *node = bb->dom[dir_index]; - gcc_assert (dom_computed[dir_index]); + gcc_checking_assert (dom_computed[dir_index]); if (!node->father) return NULL; @@ -712,14 +712,14 @@ get_immediate_dominator (enum cdi_direct /* Set the immediate dominator of the block possibly removing existing edge. NULL can be used to remove any edge. */ -void +inline void set_immediate_dominator (enum cdi_direction dir, basic_block bb, basic_block dominated_by) { unsigned int dir_index = dom_convert_dir_to_idx (dir); struct et_node *node = bb->dom[dir_index]; - gcc_assert (dom_computed[dir_index]); + gcc_checking_assert (dom_computed[dir_index]); if (node->father) {