From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 11046 invoked by alias); 20 Mar 2012 14:20:00 -0000 Received: (qmail 11033 invoked by uid 22791); 20 Mar 2012 14:19:58 -0000 X-SWARE-Spam-Status: No, hits=-5.8 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_HI,T_RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from cantor2.suse.de (HELO mx2.suse.de) (195.135.220.15) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Tue, 20 Mar 2012 14:19:35 +0000 Received: from relay1.suse.de (unknown [195.135.220.254]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by mx2.suse.de (Postfix) with ESMTP id 17D45906EB; Tue, 20 Mar 2012 15:19:34 +0100 (CET) Date: Tue, 20 Mar 2012 14:20:00 -0000 From: Richard Guenther To: Tristan Gingold Cc: GCC Patches , Eric Botcazou , Jan Hubicka Subject: Re: [Patch/cfgexpand]: also consider assembler_name to call expand_main_function In-Reply-To: <20FDC949-4630-403F-94E9-7F75B6E5E120@adacore.com> Message-ID: References: <875C6F56-7161-48DF-91FB-3A01891F8289@adacore.com> <6BCCAF3B-005E-449E-917C-4B88B78F1946@adacore.com> <20FDC949-4630-403F-94E9-7F75B6E5E120@adacore.com> User-Agent: Alpine 2.00 (LNX 1167 2008-08-23) MIME-Version: 1.0 Content-Type: MULTIPART/MIXED; BOUNDARY="8323584-676130649-1332253174=:4662" 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: 2012-03/txt/msg01354.txt.bz2 This message is in MIME format. The first part should be readable text, while the remaining parts are likely unreadable without MIME-aware tools. --8323584-676130649-1332253174=:4662 Content-Type: TEXT/PLAIN; charset=windows-1252 Content-Transfer-Encoding: 8BIT Content-length: 4065 On Tue, 20 Mar 2012, Tristan Gingold wrote: > > On Mar 20, 2012, at 1:21 PM, Richard Guenther wrote: > > > On Tue, 20 Mar 2012, Tristan Gingold wrote: > > > >> > >> On Mar 15, 2012, at 10:37 AM, Richard Guenther wrote: > >> > >>> On Wed, 14 Mar 2012, Tristan Gingold wrote: > >> […] > >> > >>> > >>> Well. To make this work in LTO the "main" function (thus, the program > >>> entry point) should be marked at cgraph level and all users of > >>> MAIN_NAME_P should instead check a flag on the cgraph node. > >>> > >>>> Will write a predicate in tree.[ch]. > >>> > >>> Please instead transition "main-ness" to the graph. > >> > >> Hi, > >> > >> here is the patch I wrote. Does it match what you had in mind ? > > > > Basically yes. Comments below. > > > >> main_identifier_node is now set in tree.c > > > > Looks good, hopefully my review-grep was as good as yours ;) > > […] > > >> diff --git a/gcc/cfgexpand.c b/gcc/cfgexpand.c > >> index bd21169..7a7a774 100644 > >> --- a/gcc/cfgexpand.c > >> +++ b/gcc/cfgexpand.c > >> @@ -4513,9 +4513,8 @@ gimple_expand_cfg (void) > >> > >> /* If this function is `main', emit a call to `__main' > >> to run global initializers, etc. */ > >> - if (DECL_NAME (current_function_decl) > >> - && MAIN_NAME_P (DECL_NAME (current_function_decl)) > >> - && DECL_FILE_SCOPE_P (current_function_decl)) > >> + if (DECL_FILE_SCOPE_P (current_function_decl) > >> + && cgraph_main_function_p (cgraph_get_node (current_function_decl))) > >> expand_main_function (); > > > > The DECL_FILE_SCOPE_P check is redundant, please remove them everywhere > > you call cgraph_main_function_p. I suppose returning false if the > > cgraph node is NULL in cgraph_main_function_p would be good. > > Ok. (I added the DECL_FILE_SCOPE_P check to avoid the cgraph lookup for speed reason) > > […] > > >> diff --git a/gcc/cgraphunit.c b/gcc/cgraphunit.c > >> index 516f187..4a59f63 100644 > >> --- a/gcc/cgraphunit.c > >> +++ b/gcc/cgraphunit.c > >> @@ -346,6 +346,10 @@ cgraph_finalize_function (tree decl, bool nested) > >> notice_global_symbol (decl); > >> node->local.finalized = true; > >> node->lowered = DECL_STRUCT_FUNCTION (decl)->cfg != NULL; > >> + node->local.main_function = > >> + DECL_FILE_SCOPE_P (decl) > >> + && ((!DECL_ASSEMBLER_NAME_SET_P (decl) && MAIN_NAME_P (DECL_NAME (decl))) > >> + || decl_assembler_name_equal (decl, main_identifier_node)); > > > > If we finalize a function we should always create an assembler name, > > thus I'd change the above to > > > > node->local.main_function = decl_assembler_name_equal (decl, > > main_identifier_node); > > Indeed. At worst, the assembler name is created during the call to notice_global_symbol. > > > btw, decl_assembler_name_equal doesn't seem to remove target-specific > > mangling - do some OSes "mangle" main differently (I'm thinking of > > leading underscores or complete renames)? Thus, I guess the > > targets might want to be able to provide the main_identifier_assember_name > > you use here. > > I think this is currently OK because decl_assembler_name_equal deals > with leading underscore correctly. I have checked that on Darwin, > which has a leading underscore. > > The only target that mangle names is i386 cygwin/mingw, which 'annotates' > stdcall and fastcall function, but main() is regular. > > But I agree this mechanism is fragile. > > In order to make this mechanism stronger, we could add main_function_node, which > designates the FUNCTION_DECL that is the main function (if not NULL_TREE), with > a fallback on main_identifier_node for regular languages such as C or C++. I'd rather get away from using a global main_identifier_node, instead make that frontend specific, and introduce targetm.main_assembler_name which the assembler-name creating langhook would make sure to use when mangling what the FE thinks main is. main_identifier_node should not serve any purpose outside of Frontends. But I see both as a possible cleanup opportunity, not a necessary change. Richard. --8323584-676130649-1332253174=:4662--