From patchwork Fri Feb 4 07:49:05 2011 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Alexandre Oliva X-Patchwork-Id: 81813 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 15B7AB710C for ; Fri, 4 Feb 2011 18:49:44 +1100 (EST) Received: (qmail 8460 invoked by alias); 4 Feb 2011 07:49:37 -0000 Received: (qmail 8220 invoked by uid 22791); 4 Feb 2011 07:49:29 -0000 X-SWARE-Spam-Status: No, hits=-5.6 required=5.0 tests=AWL, BAYES_00, RCVD_IN_DNSWL_HI, SPF_HELO_PASS, T_RP_MATCHES_RCVD, T_TVD_MIME_NO_HEADERS X-Spam-Check-By: sourceware.org Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Fri, 04 Feb 2011 07:49:16 +0000 Received: from int-mx02.intmail.prod.int.phx2.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) by mx1.redhat.com (8.13.8/8.13.8) with ESMTP id p147nCf8018547 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Fri, 4 Feb 2011 02:49:13 -0500 Received: from freie.oliva.athome.lsd.ic.unicamp.br (ovpn01.gateway.prod.ext.phx2.redhat.com [10.5.9.1]) by int-mx02.intmail.prod.int.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id p147nBna032323 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Fri, 4 Feb 2011 02:49:12 -0500 Received: from livre.localdomain (livre-to-gw.oliva.athome.lsd.ic.unicamp.br [172.31.160.19]) by freie.oliva.athome.lsd.ic.unicamp.br (8.14.4/8.14.4) with ESMTP id p147n9Uk019559; Fri, 4 Feb 2011 05:49:10 -0200 Received: from livre.localdomain (aoliva@localhost.localdomain [127.0.0.1]) by livre.localdomain (8.14.3/8.14.3/Debian-5+lenny1) with ESMTP id p147n7GF009764; Fri, 4 Feb 2011 05:49:08 -0200 Received: (from aoliva@localhost) by livre.localdomain (8.14.3/8.14.3/Submit) id p147n6rV009762; Fri, 4 Feb 2011 05:49:06 -0200 From: Alexandre Oliva To: Richard Guenther Cc: gcc-patches@gcc.gnu.org, Jan Hubicka Subject: Re: [PR debug/47106] account used vars only once References: Date: Fri, 04 Feb 2011 05:49:05 -0200 In-Reply-To: (Richard Guenther's message of "Thu, 3 Feb 2011 11:19:42 +0100") Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/23.1 (gnu/linux) MIME-Version: 1.0 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 Feb 3, 2011, Richard Guenther wrote: > On Thu, Feb 3, 2011 at 6:50 AM, Alexandre Oliva wrote: >> On Feb  2, 2011, Alexandre Oliva wrote: >> >>> Perhaps relying on the used flag and going over all scope blocks wasn't >>> such a good idea, after all; I suppose we could get something more >>> reliable using referenced_vars only.  Although I'm finding it difficult >>> to figure out whether presence in referenced_vars should ever be >>> different from having the used flag marked, it appears that presence in >>> referenced_vars is maintained more accurately, even during inlining. >> >> Indeed, early in compilation we don't have referenced_vars at all, but >> we can make do without them, going through the scope blocks. > I think we can simply move pass_inline_parameters to after > pass_referenced_vars. That isn't enough. We often attempt to estimate the stack size of a function before we as much as put it in SSA form (let alone compute referenced_vras), while processing other functions. > referenced_var_lookup_in should rather take a struct function argument > than a hashtable (in fact, given the few existing callers I'd just change > the referenced_var_lookup signature). *nod* > Can you do a quick comparison between the old and the new > stack frame estimations on some random files from gcc? I guess... Once we have another working patch. My previous patch did away with the need for rearranging passes by using the current computation when referenced_vars are not available, but if we switch to referenced_vars only, we have to figure out how to rearrange the passes so that we make reasonable estimates. Here's where I stand ATM. The first patch is supposed to implement the changes you suggested, plus some attempts to fix things up or otherwise detect problems. It finds too many. The second patch restores some of the removed functionality to error out if the stack size computations differ. None of these are meant for inclusion, I post them for the record in case any of you guys would like to pick it up while I get some sleep. Suggestions on how to proceed are welcome. I'm thinking of rearranging the passes so we compute referenced_vars for all functions before advancing to other passes for any other functions, but I have no idea of how to do that. I'll dig it up if nobody beats me to it. Index: gcc/cfgexpand.c =================================================================== --- gcc/cfgexpand.c.orig 2011-02-04 05:26:40.137008650 -0200 +++ gcc/cfgexpand.c 2011-02-04 05:27:40.951213575 -0200 @@ -1311,6 +1311,29 @@ create_stack_guard (void) crtl->stack_protect_guard = guard; } +/* A subroutine of expand_used_vars. Walk down through the BLOCK tree + expanding variables. Those variables that can be put into registers + are allocated pseudos; those that can't are put on the stack. + + TOPLEVEL is true if this is the outermost BLOCK. */ + +static HOST_WIDE_INT +account_used_vars_for_block (tree block, bool toplevel) +{ + tree t; + HOST_WIDE_INT size = 0; + + /* Expand all variables at this level. */ + for (t = BLOCK_VARS (block); t ; t = DECL_CHAIN (t)) + size += expand_one_var (t, toplevel, false); + + /* Expand all variables at containing levels. */ + for (t = BLOCK_SUBBLOCKS (block); t ; t = BLOCK_CHAIN (t)) + size += account_used_vars_for_block (t, false); + + return size; +} + /* Prepare for expanding variables. */ static void init_vars_expansion (void) @@ -1354,7 +1377,10 @@ HOST_WIDE_INT estimated_stack_frame_size (tree decl) { HOST_WIDE_INT size = 0; + HOST_WIDE_INT osize = 0; + unsigned ix; size_t i; + tree outer_block = DECL_INITIAL (current_function_decl); tree var; tree old_cur_fun_decl = current_function_decl; referenced_var_iterator rvi; @@ -1378,6 +1404,11 @@ estimated_stack_frame_size (tree decl) size += account_stack_vars (); fini_vars_expansion (); } + + if (size != osize) + error ("est stack size changed from %li to %li", + (long)osize, (long)size); + pop_cfun (); current_function_decl = old_cur_fun_decl; return size;