From patchwork Thu Feb 3 05:50:46 2011 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Alexandre Oliva X-Patchwork-Id: 81619 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 315FEB70EB for ; Thu, 3 Feb 2011 16:51:51 +1100 (EST) Received: (qmail 9887 invoked by alias); 3 Feb 2011 05:51:50 -0000 Received: (qmail 9875 invoked by uid 22791); 3 Feb 2011 05:51:49 -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; Thu, 03 Feb 2011 05:51:44 +0000 Received: from int-mx01.intmail.prod.int.phx2.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id p135penH027315 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Thu, 3 Feb 2011 00:51:40 -0500 Received: from freie.oliva.athome.lsd.ic.unicamp.br (ovpn01.gateway.prod.ext.phx2.redhat.com [10.5.9.1]) by int-mx01.intmail.prod.int.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id p135pKwG024472 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Thu, 3 Feb 2011 00:51:33 -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 p135pArP021267; Thu, 3 Feb 2011 03:51:14 -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 p135p0LD028393; Thu, 3 Feb 2011 03:51:02 -0200 Received: (from aoliva@localhost) by livre.localdomain (8.14.3/8.14.3/Submit) id p135olPn028390; Thu, 3 Feb 2011 03:50:47 -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: Thu, 03 Feb 2011 03:50:46 -0200 In-Reply-To: (Alexandre Oliva's message of "Wed, 02 Feb 2011 17:34:57 -0200") 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 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. Something very close to this patch (I'd accidentally left init_vars_expansion() out) regstrapped without regressions on x86_64-linux-gnu and i686-pc-linux-gnu. I'm now retesting this. Ok if it passes? for gcc/ChangeLog from Alexandre Oliva PR debug/47106 PR debug/47402 * cfgexpand.c (account_used_vars_for_block): Disregard used flag. (estimated_stack_frame_size): Prefer referenced vars over scope block vars. * tree-flow.h (referenced_var_lookup_in): Declare. * tree-inline.c (remap_decl): Only mark VAR_DECLs as referenced that were referenced in the original function. (copy_decl_for_dup_finish): Likewise. * tree-dfa.c (referenced_var_lookup_in): Split out of... (referenced_var_lookup): ... this. for gcc/testsuite/ChangeLog from Alexandre Oliva PR debug/47106 PR debug/47402 * g++.dg/debug/pr47106.C: New. Index: gcc/cfgexpand.c =================================================================== --- gcc/cfgexpand.c.orig 2011-02-02 19:58:10.013374804 -0200 +++ gcc/cfgexpand.c 2011-02-02 20:21:51.148296315 -0200 @@ -1325,8 +1325,7 @@ account_used_vars_for_block (tree block, /* Expand all variables at this level. */ for (t = BLOCK_VARS (block); t ; t = DECL_CHAIN (t)) - if (var_ann (t) && is_used_p (t)) - size += expand_one_var (t, toplevel, false); + size += expand_one_var (t, toplevel, false); /* Expand all variables at containing levels. */ for (t = BLOCK_SUBBLOCKS (block); t ; t = BLOCK_CHAIN (t)) @@ -1381,20 +1380,38 @@ estimated_stack_frame_size (tree decl) size_t i; tree var, outer_block = DECL_INITIAL (current_function_decl); unsigned ix; + referenced_var_iterator rvi; tree old_cur_fun_decl = current_function_decl; + current_function_decl = decl; push_cfun (DECL_STRUCT_FUNCTION (decl)); - init_vars_expansion (); - - FOR_EACH_LOCAL_DECL (cfun, ix, var) + /* We want to count only referenced vars, but if asked to estimate + stack size before we compute them, guess based on local decls and + scopes. We have to make sure we compute the same values for + debug and non-debug compilations, in spite of the removal of + unused variables from lexical blocks, but this removal never + occurs before referenced vars are computed. Even when we perform + inlining and versioning, we register referenced vars, and can + thus use them for further stack size estimation. */ + if (gimple_in_ssa_p (cfun)) + { + gcc_checking_assert (gimple_referenced_vars (cfun)); + FOR_EACH_REFERENCED_VAR (var, rvi) + size += expand_one_var (var, true, false); + } + else { - /* TREE_USED marks local variables that do not appear in lexical - blocks. We don't want to expand those that do twice. */ - if (TREE_USED (var)) - size += expand_one_var (var, true, false); + FOR_EACH_LOCAL_DECL (cfun, ix, var) + { + /* TREE_USED marks local variables that do not appear in + lexical blocks. We don't want to expand those that do + twice. */ + if (TREE_USED (var)) + size += expand_one_var (var, true, false); + } + size += account_used_vars_for_block (outer_block, true); } - size += account_used_vars_for_block (outer_block, true); if (stack_vars_num > 0) { Index: gcc/tree-flow.h =================================================================== --- gcc/tree-flow.h.orig 2011-02-02 17:31:40.430302181 -0200 +++ gcc/tree-flow.h 2011-02-02 20:05:17.169414197 -0200 @@ -319,6 +319,7 @@ typedef struct !end_referenced_vars_p (&(ITER)); \ (VAR) = next_referenced_var (&(ITER))) +extern tree referenced_var_lookup_in (htab_t, unsigned int); extern tree referenced_var_lookup (unsigned int); extern bool referenced_var_check_and_insert (tree); #define num_referenced_vars htab_elements (gimple_referenced_vars (cfun)) Index: gcc/tree-inline.c =================================================================== --- gcc/tree-inline.c.orig 2011-02-02 17:31:41.279276897 -0200 +++ gcc/tree-inline.c 2011-02-03 00:08:30.100962123 -0200 @@ -317,7 +317,12 @@ remap_decl (tree decl, copy_body_data *i || TREE_CODE (t) == RESULT_DECL || TREE_CODE (t) == PARM_DECL)) { get_var_ann (t); - add_referenced_var (t); + if (TREE_CODE (decl) != VAR_DECL + || !gimple_referenced_vars (DECL_STRUCT_FUNCTION (id->src_fn)) + || referenced_var_lookup_in (gimple_referenced_vars + (DECL_STRUCT_FUNCTION (id->src_fn)), + DECL_UID (decl))) + add_referenced_var (t); } return t; } @@ -4753,6 +4758,14 @@ copy_decl_for_dup_finish (copy_body_data new function. */ DECL_CONTEXT (copy) = id->dst_fn; + if (TREE_CODE (decl) == VAR_DECL + && gimple_referenced_vars (cfun) + && gimple_referenced_vars (DECL_STRUCT_FUNCTION (id->src_fn)) + && referenced_var_lookup_in (gimple_referenced_vars + (DECL_STRUCT_FUNCTION (id->src_fn)), + DECL_UID (decl))) + add_referenced_var (copy); + return copy; } Index: gcc/tree-dfa.c =================================================================== --- gcc/tree-dfa.c.orig 2011-02-02 16:21:12.934940723 -0200 +++ gcc/tree-dfa.c 2011-02-02 20:05:17.249411771 -0200 @@ -490,10 +490,18 @@ find_referenced_vars_in (gimple stmt) tree referenced_var_lookup (unsigned int uid) { + return referenced_var_lookup_in (gimple_referenced_vars (cfun), uid); +} + +/* Lookup UID in REFVARS and return the associated variable. */ + +tree +referenced_var_lookup_in (htab_t refvars, unsigned int uid) +{ tree h; struct tree_decl_minimal in; in.uid = uid; - h = (tree) htab_find_with_hash (gimple_referenced_vars (cfun), &in, uid); + h = (tree) htab_find_with_hash (refvars, &in, uid); return h; } Index: gcc/testsuite/g++.dg/debug/pr47106.C =================================================================== --- /dev/null 1970-01-01 00:00:00.000000000 +0000 +++ gcc/testsuite/g++.dg/debug/pr47106.C 2011-02-02 20:18:16.826787838 -0200 @@ -0,0 +1,37 @@ +// { dg-do compile } +// { dg-options "-O -fpartial-inlining -flto -fconserve-stack -fcompare-debug" } + +void end (int, int) __attribute__ ((__noreturn__)); + +struct S +{ + int i; + S *s; +}; + +inline bool f (S *s) +{ + if (!s->s) + end (0, 0); + return s->s == s; +} + +inline bool +baz (S s1, S) +{ + while (f (&s1)); +} + +inline bool +bar (S s1, S s2, S) +{ + baz (s1, s2); +} + +S getS (); + +bool +foo () +{ + bar (getS (), getS (), getS ()); +}