From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 15821 invoked by alias); 13 Nov 2010 23:32:30 -0000 Received: (qmail 15811 invoked by uid 22791); 13 Nov 2010 23:32:29 -0000 X-SWARE-Spam-Status: No, hits=-1.9 required=5.0 tests=AWL,BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,FREEMAIL_FROM,RCVD_IN_DNSWL_NONE X-Spam-Check-By: sourceware.org Received: from mail-vw0-f47.google.com (HELO mail-vw0-f47.google.com) (209.85.212.47) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Sat, 13 Nov 2010 23:31:33 +0000 Received: by vws12 with SMTP id 12so1338523vws.20 for ; Sat, 13 Nov 2010 15:31:31 -0800 (PST) MIME-Version: 1.0 Received: by 10.220.201.141 with SMTP id fa13mr956744vcb.169.1289691090957; Sat, 13 Nov 2010 15:31:30 -0800 (PST) Received: by 10.220.118.12 with HTTP; Sat, 13 Nov 2010 15:31:30 -0800 (PST) In-Reply-To: References: <20101026141604.GD20390@kam.mff.cuni.cz> Date: Sun, 14 Nov 2010 00:18:00 -0000 Message-ID: Subject: Re: Fix removal of ctors/dtors From: "H.J. Lu" To: Jan Hubicka Cc: gcc-patches@gcc.gnu.org Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable X-IsSubscribed: yes Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org X-SW-Source: 2010-11/txt/msg01450.txt.bz2 On Mon, Nov 8, 2010 at 11:03 AM, H.J. Lu wrote: > On Tue, Oct 26, 2010 at 7:16 AM, Jan Hubicka wrote: >> Hi, >> while implementing code removing pure & const constructors I considered = correct option of >> dropping CONSTRUCTOR/DESTRUCTOR flag for functions found to be const or = pure or slightly hacky >> of simply teaching dead function removal to consider them dead. =A0The s= econd is suboptimal >> for static constructors & destructors that are also called dirrectly. = =A0I concluded that no one >> does that, but Zdenek did and it triggers one overparanoid sanity check. >> >> So this patch takes the cleaner route of just dropping the flags and let= ting >> unreachable code removal to remove the functions later. =A0This happens = in >> visibility pass where we sanitize flags for pure/const constructors comm= ing >> from forntends or via cgraph_set_pure_flag/cgraph_set_const_flags when t= hey are >> discovered pure/const later. >> >> While working on it I also noticed that we don't handle correctly extern >> declarations (we consider them extern inlines and keep them in the cgrap= h. >> This is wasteful especially for LTO) and extern inlines (we keep them in= the >> program even if they are obviously dead - they are never called nor have= address >> taken up to the IPA inliner). Cleaning this issue shown that both inline= r and >> ipa-cp compute code size costs incorrectly by assuming that optimizing o= ut >> external copy of extern function it will save code size. >> >> Finally both inliner and ipa-cp confuse handling of functions with addre= ss >> taken. =A0ipa-cp is considering functions with address taken to be cloni= ng candidates. >> This is wrong. =A0We should clone only when we think original function i= s not called at all >> but we can't say for sure since it is externally visible. =A0Clonning is= of course possible, >> but current impementation leads to wrong code when updating call sites o= f calls promoted to >> be direct. >> >> Inliner on the other hand produce invalid callgraph with inline clones h= aving >> address taken. >> >> Bootstrapped/regtested x86_64-linux, lto-bootstrapped x86_64-linux and t= ested >> with Firefox builds. =A0 I also tested that tramp3d does not care about = the >> inliner mertric fixes. >> >> Honza >> >> =A0 =A0 =A0 =A0* cgraph.c (cgraph_set_readonly_flag): Rename to... >> =A0 =A0 =A0 =A0(cgraph_set_const_flags) ... this one; get also looping a= rgument; >> =A0 =A0 =A0 =A0clear constructor/destructor flags. >> =A0 =A0 =A0 =A0(cgraph_set_pure_flag): Likewise. >> =A0 =A0 =A0 =A0(cgraph_set_looping_const_or_pure_flag): Remove. >> =A0 =A0 =A0 =A0(cgraph_can_remove_if_no_direct_calls_and_refs): Do not t= ry >> =A0 =A0 =A0 =A0to optimize away static ctors/dtors; it does not work on = inline clones; >> =A0 =A0 =A0 =A0external functions can always be rmeoved. >> =A0 =A0 =A0 =A0(cgraph_will_be_removed_from_program_if_no_direct_calls):= Assert on inline >> =A0 =A0 =A0 =A0clones; in LTO external functions always can go. >> =A0 =A0 =A0 =A0(cgraph_used_from_object_file_p): Handle EXTERNAL functio= ns correctly. >> =A0 =A0 =A0 =A0(cgraph_mark_address_taken_node): Assert that we are not = taking address of >> =A0 =A0 =A0 =A0inline clone. >> =A0 =A0 =A0 =A0(cgraph_can_remove_if_no_direct_calls_p): We always event= ually remove >> =A0 =A0 =A0 =A0external functions. >> =A0 =A0 =A0 =A0* ipa-cp.c (ipcp_cloning_candidate_p): Do not clone funct= ions with address taken. >> =A0 =A0 =A0 =A0(ipcp_initialize_node_lattices): Only local functions can= be handled without cloning. >> =A0 =A0 =A0 =A0* cgraph.h (cgraph_set_readonly_flag, >> =A0 =A0 =A0 =A0cgraph_set_looping_const_or_pure_flag): Remove. >> =A0 =A0 =A0 =A0(cgraph_set_const_flag): Declare. >> =A0 =A0 =A0 =A0(cgraph_set_pure_flag): Update. >> =A0 =A0 =A0 =A0* ipa-pure-const (propagate_pure_const, local_pure_const)= : Update >> =A0 =A0 =A0 =A0flags setting code. >> =A0 =A0 =A0 =A0* ipa.c (cgraph_remove_unreachable_nodes): Fix formating;= do not look at inline >> =A0 =A0 =A0 =A0clones; fix handling of external definitions. >> =A0 =A0 =A0 =A0(cgraph_postorder): Do not look at inline clones in the f= irst pass. >> =A0 =A0 =A0 =A0(function_and_variable_visibility): Drop constructors/des= tructor >> =A0 =A0 =A0 =A0flags at pure and const functions. >> =A0 =A0 =A0 =A0* tree-profile.c (tree_profiling): Update. >> =A0 =A0 =A0 =A0* ipa-inline.c (cgraph_clone_inlined_nodes): Always clone= functions with >> =A0 =A0 =A0 =A0address taken; external functions do not account to whole= program size. >> =A0 =A0 =A0 =A0(cgraph_decide_inlining): Likewise; do not try to inline = functions already >> =A0 =A0 =A0 =A0inlined. >> >> =A0 =A0 =A0 =A0* testsuite/gcc.dg/lto/pr45736_0.c: New function. > > This patch caused: > > http://gcc.gnu.org/bugzilla/show_bug.cgi?id=3D46367 > This also caused: http://gcc.gnu.org/bugzilla/show_bug.cgi?id=3D46469 --=20 H.J.