From patchwork Tue Jan 10 19:27:15 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Eric Botcazou X-Patchwork-Id: 135297 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 88C1BB6FCC for ; Wed, 11 Jan 2012 06:30:43 +1100 (EST) Comment: DKIM? See http://www.dkim.org DKIM-Signature: v=1; a=rsa-sha1; c=relaxed/relaxed; d=gcc.gnu.org; s=default; x=1326828645; h=Comment: DomainKey-Signature:Received:Received:Received:Received:Received: Received:From:To:Subject:Date:User-Agent:MIME-Version: Content-Disposition:Content-Type:Message-Id:Mailing-List: Precedence:List-Id:List-Unsubscribe:List-Archive:List-Post: List-Help:Sender:Delivered-To; bh=We8M4EeA7gKMe5rmjuTPBa9S7kM=; b=nm2Fk2uqenQDYU+o2PEwg4Odt09WpjWoPCtTwUgMrj6at0fABQKDW1cUpGE2K+ MTt7woJrFXeMlqyzJOimuKEQwFqYyhIBms/VEqzQAsQRVzN0SPKeinIhhQMB2RHe xEpPn876parbfLHU5jGos2OJxRFSnqlaRP6xgcdQ+2E4Q= Comment: DomainKeys? See http://antispam.yahoo.com/domainkeys DomainKey-Signature: a=rsa-sha1; q=dns; c=nofws; s=default; d=gcc.gnu.org; h=Received:Received:X-SWARE-Spam-Status:X-Spam-Check-By:Received:Received:Received:Received:From:To:Subject:Date:User-Agent:MIME-Version:Content-Disposition:Content-Type:Message-Id:Mailing-List:Precedence:List-Id:List-Unsubscribe:List-Archive:List-Post:List-Help:Sender:Delivered-To; b=VayiFg1G+JqhDz6Sldj66maytoqfoX44HnY2qxTe092JLCAEUQyDMDWQL2/c2f IvjMR+/2Q8gbLmVm45vAByj4pLlwDf8zuWNyQSEos93DTqAif4FAWvUSahMR1v1T t0V4bMinH8Cy2+0yAvlkEyaN/hj94AEJld1t0Wt53PjsQ=; Received: (qmail 29597 invoked by alias); 10 Jan 2012 19:30:36 -0000 Received: (qmail 29563 invoked by uid 22791); 10 Jan 2012 19:30:30 -0000 X-SWARE-Spam-Status: No, hits=-1.9 required=5.0 tests=AWL,BAYES_00,TW_FN X-Spam-Check-By: sourceware.org Received: from mel.act-europe.fr (HELO mel.act-europe.fr) (194.98.77.210) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Tue, 10 Jan 2012 19:30:15 +0000 Received: from localhost (localhost [127.0.0.1]) by filtered-smtp.eu.adacore.com (Postfix) with ESMTP id A8FB9CB3055 for ; Tue, 10 Jan 2012 20:30:14 +0100 (CET) Received: from mel.act-europe.fr ([127.0.0.1]) by localhost (smtp.eu.adacore.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id GaeXI6vfXotE for ; Tue, 10 Jan 2012 20:30:14 +0100 (CET) Received: from [192.168.1.2] (bon31-6-88-161-99-133.fbx.proxad.net [88.161.99.133]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by mel.act-europe.fr (Postfix) with ESMTP id 3369CCB304A for ; Tue, 10 Jan 2012 20:30:14 +0100 (CET) From: Eric Botcazou To: gcc-patches@gcc.gnu.org Subject: [patch] Fix ICEs with functions returning variable-sized array Date: Tue, 10 Jan 2012 20:27:15 +0100 User-Agent: KMail/1.9.9 MIME-Version: 1.0 Content-Disposition: inline Message-Id: <201201102027.15802.ebotcazou@adacore.com> 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 This is a couple of regressions present on the mainline. For the first testcase at O2 -gnatn: +===========================GNAT BUG DETECTED==============================+ | 4.7.0 20120102 (experimental) [trunk revision 182780] (i586-suse-linux) GCC error:| | in assign_stack_temp_for_type, at function.c:796 | | Error detected around p1.adb:3:4 For the second testcase: +===========================GNAT BUG DETECTED==============================+ | 4.7.0 20120102 (experimental) [trunk revision 182780] (i586-suse-linux) GCC error:| | in declare_return_variable, at tree-inline.c:2904 | | Error detected around p2.adb:3:4 Both are caused by the fnsplit IPA pass being run on a function returning a variable-sized array. In both cases, the part that isn't inlined is made up of a single "raise" statement, i.e. a no-return call. So fnsplit rewrites the call statement into just: f.part (arguments); In the first case, the compilation aborts when the RTL expander attempts to create a temporary for the return value (which would have variable size) while, in the second case, it aborts on the assertion: gcc_assert (TREE_CODE (TYPE_SIZE_UNIT (callee_type)) == INTEGER_CST); when the inliner attemps to inline the part that wasn't inlined(!). The proposed fix is to turn the part that isn't inlined into a function that returns void. This involves straightforward adjustments to the two versioning machineries (cgraph and tree). Tested on i586-suse-linux, OK for the mainline? 2012-01-10 Eric Botcazou * tree.h (build_function_decl_skip_args): Add boolean parameter. (build_function_type_skip_args): Delete. * tree.c (build_function_type_skip_args): Make static and add SKIP_RETURN parameter. Fix thinko in the handling of variants. (build_function_decl_skip_args): Add SKIP_RETURN parameter and pass it to build_function_type_skip_args. * cgraph.h (cgraph_function_versioning): Add boolean parameter. (tree_function_versioning): Likewise. * cgraph.c (cgraph_create_virtual_clone): Adjust call to build_function_decl_skip_args. * cgraphunit.c (cgraph_function_versioning): Add SKIP_RETURN parameter and pass it to build_function_decl_skip_args/tree_function_versioning. (cgraph_materialize_clone): Adjust call to tree_function_versioning. * ipa-inline-transform.c (save_inline_function_body): Likewise. * trans-mem.c (ipa_tm_create_version): Likewise. * tree-sra.c (modify_function): Likewise for cgraph_function_versioning. * tree-inline.c (declare_return_variable): Remove always-true test. (tree_function_versioning): Add SKIP_RETURN parameter. If the function returns non-void and SKIP_RETURN, create a void-typed RESULT_DECL. * ipa-split.c (split_function): Skip the return value for the split part if it doesn't return. 2012-01-10 Eric Botcazou * gnat.dg/opt23.ad[sb]: New test. * gnat.dg/opt23_pkg.ad[sb]: New helper. * gnat.dg/opt24.ad[sb]: New test. Index: tree.h =================================================================== --- tree.h (revision 182780) +++ tree.h (working copy) @@ -4386,8 +4386,7 @@ extern tree build_nonshared_array_type ( extern tree build_array_type_nelts (tree, unsigned HOST_WIDE_INT); extern tree build_function_type (tree, tree); extern tree build_function_type_list (tree, ...); -extern tree build_function_type_skip_args (tree, bitmap); -extern tree build_function_decl_skip_args (tree, bitmap); +extern tree build_function_decl_skip_args (tree, bitmap, bool); extern tree build_varargs_function_type_list (tree, ...); extern tree build_function_type_array (tree, int, tree *); extern tree build_varargs_function_type_array (tree, int, tree *); Index: tree.c =================================================================== --- tree.c (revision 182780) +++ tree.c (working copy) @@ -7556,10 +7556,12 @@ build_function_type (tree value_type, tr return t; } -/* Build variant of function type ORIG_TYPE skipping ARGS_TO_SKIP. */ +/* Build variant of function type ORIG_TYPE skipping ARGS_TO_SKIP and the + return value if SKIP_RETURN is true. */ -tree -build_function_type_skip_args (tree orig_type, bitmap args_to_skip) +static tree +build_function_type_skip_args (tree orig_type, bitmap args_to_skip, + bool skip_return) { tree new_type = NULL; tree args, new_args = NULL, t; @@ -7599,11 +7601,15 @@ build_function_type_skip_args (tree orig TYPE_CONTEXT (new_type) = TYPE_CONTEXT (orig_type); } + if (skip_return) + TREE_TYPE (new_type) = void_type_node; + /* This is a new type, not a copy of an old type. Need to reassociate variants. We can handle everything except the main variant lazily. */ t = TYPE_MAIN_VARIANT (orig_type); - if (orig_type != t) + if (t != orig_type) { + t = build_function_type_skip_args (t, args_to_skip, skip_return); TYPE_MAIN_VARIANT (new_type) = t; TYPE_NEXT_VARIANT (new_type) = TYPE_NEXT_VARIANT (t); TYPE_NEXT_VARIANT (t) = new_type; @@ -7613,24 +7619,29 @@ build_function_type_skip_args (tree orig TYPE_MAIN_VARIANT (new_type) = new_type; TYPE_NEXT_VARIANT (new_type) = NULL; } + return new_type; } -/* Build variant of function type ORIG_TYPE skipping ARGS_TO_SKIP. +/* Build variant of function decl ORIG_DECL skipping ARGS_TO_SKIP and the + return value if SKIP_RETURN is true. Arguments from DECL_ARGUMENTS list can't be removed now, since they are linked by TREE_CHAIN directly. The caller is responsible for eliminating them when they are being duplicated (i.e. copy_arguments_for_versioning). */ tree -build_function_decl_skip_args (tree orig_decl, bitmap args_to_skip) +build_function_decl_skip_args (tree orig_decl, bitmap args_to_skip, + bool skip_return) { tree new_decl = copy_node (orig_decl); tree new_type; new_type = TREE_TYPE (orig_decl); - if (prototype_p (new_type)) - new_type = build_function_type_skip_args (new_type, args_to_skip); + if (prototype_p (new_type) + || (skip_return && !VOID_TYPE_P (TREE_TYPE (new_type)))) + new_type + = build_function_type_skip_args (new_type, args_to_skip, skip_return); TREE_TYPE (new_decl) = new_type; /* For declarations setting DECL_VINDEX (i.e. methods) Index: cgraph.h =================================================================== --- cgraph.h (revision 182780) +++ cgraph.h (working copy) @@ -580,10 +580,10 @@ struct cgraph_node * cgraph_copy_node_fo struct cgraph_node *cgraph_function_versioning (struct cgraph_node *, VEC(cgraph_edge_p,heap)*, VEC(ipa_replace_map_p,gc)*, - bitmap, bitmap, basic_block, - const char *); -void tree_function_versioning (tree, tree, VEC (ipa_replace_map_p,gc)*, bool, bitmap, - bitmap, basic_block); + bitmap, bool, bitmap, + basic_block, const char *); +void tree_function_versioning (tree, tree, VEC (ipa_replace_map_p,gc)*, + bool, bitmap, bool, bitmap, basic_block); void record_references_in_initializer (tree, bool); bool cgraph_process_new_functions (void); void cgraph_process_same_body_aliases (void); Index: cgraph.c =================================================================== --- cgraph.c (revision 182780) +++ cgraph.c (working copy) @@ -2246,7 +2246,7 @@ cgraph_create_virtual_clone (struct cgra if (!args_to_skip) new_decl = copy_node (old_decl); else - new_decl = build_function_decl_skip_args (old_decl, args_to_skip); + new_decl = build_function_decl_skip_args (old_decl, args_to_skip, false); DECL_STRUCT_FUNCTION (new_decl) = NULL; /* Generate a new name for the new version. */ Index: cgraphunit.c =================================================================== --- cgraphunit.c (revision 182780) +++ cgraphunit.c (working copy) @@ -2333,17 +2333,21 @@ cgraph_copy_node_for_versioning (struct TREE_MAP is a mapping of tree nodes we want to replace with new ones (according to results of prior analysis). OLD_VERSION_NODE is the node that is versioned. - It returns the new version's cgraph node. + If non-NULL ARGS_TO_SKIP determine function parameters to remove from new version. + If SKIP_RETURN is true, the new version will return void. If non-NULL BLOCK_TO_COPY determine what basic blocks to copy. - If non_NULL NEW_ENTRY determine new entry BB of the clone. */ + If non_NULL NEW_ENTRY determine new entry BB of the clone. + + Return the new version's cgraph node. */ struct cgraph_node * cgraph_function_versioning (struct cgraph_node *old_version_node, VEC(cgraph_edge_p,heap) *redirect_callers, VEC (ipa_replace_map_p,gc)* tree_map, bitmap args_to_skip, + bool skip_return, bitmap bbs_to_copy, basic_block new_entry_block, const char *clone_name) @@ -2357,12 +2361,12 @@ cgraph_function_versioning (struct cgrap gcc_assert (old_version_node->local.can_change_signature || !args_to_skip); - /* Make a new FUNCTION_DECL tree node for the - new version. */ - if (!args_to_skip) + /* Make a new FUNCTION_DECL tree node for the new version. */ + if (!args_to_skip && !skip_return) new_decl = copy_node (old_decl); else - new_decl = build_function_decl_skip_args (old_decl, args_to_skip); + new_decl + = build_function_decl_skip_args (old_decl, args_to_skip, skip_return); /* Generate a new name for the new version. */ DECL_NAME (new_decl) = clone_function_name (old_decl, clone_name); @@ -2381,7 +2385,7 @@ cgraph_function_versioning (struct cgrap /* Copy the OLD_VERSION_NODE function tree to the new version. */ tree_function_versioning (old_decl, new_decl, tree_map, false, args_to_skip, - bbs_to_copy, new_entry_block); + skip_return, bbs_to_copy, new_entry_block); /* Update the new version's properties. Make The new version visible only within this translation unit. Make sure @@ -2412,7 +2416,8 @@ cgraph_materialize_clone (struct cgraph_ /* Copy the OLD_VERSION_NODE function tree to the new version. */ tree_function_versioning (node->clone_of->decl, node->decl, node->clone.tree_map, true, - node->clone.args_to_skip, NULL, NULL); + node->clone.args_to_skip, false, + NULL, NULL); if (cgraph_dump_file) { dump_function_to_file (node->clone_of->decl, cgraph_dump_file, dump_flags); Index: ipa-inline-transform.c =================================================================== --- ipa-inline-transform.c (revision 182780) +++ ipa-inline-transform.c (working copy) @@ -324,7 +324,7 @@ save_inline_function_body (struct cgraph /* Copy the OLD_VERSION_NODE function tree to the new version. */ tree_function_versioning (node->decl, first_clone->decl, NULL, true, NULL, - NULL, NULL); + false, NULL, NULL); /* The function will be short lived and removed after we inline all the clones, but make it internal so we won't confuse ourself. */ Index: ipa-split.c =================================================================== --- ipa-split.c (revision 182780) +++ ipa-split.c (working copy) @@ -1098,6 +1098,7 @@ split_function (struct split_point *spli /* Now create the actual clone. */ rebuild_cgraph_edges (); node = cgraph_function_versioning (cur_node, NULL, NULL, args_to_skip, + !split_part_return_p, split_point->split_bbs, split_point->entry_bb, "part"); /* For usual cloning it is enough to clear builtin only when signature Index: trans-mem.c =================================================================== --- trans-mem.c (revision 182780) +++ trans-mem.c (working copy) @@ -4263,7 +4263,7 @@ ipa_tm_create_version (struct cgraph_nod DECL_WEAK (new_decl) = 0; } - tree_function_versioning (old_decl, new_decl, NULL, false, NULL, + tree_function_versioning (old_decl, new_decl, NULL, false, NULL, false, NULL, NULL); } Index: tree-sra.c =================================================================== --- tree-sra.c (revision 182932) +++ tree-sra.c (working copy) @@ -4700,7 +4700,7 @@ modify_function (struct cgraph_node *nod current_function_decl = NULL_TREE; new_node = cgraph_function_versioning (node, redirect_callers, NULL, NULL, - NULL, NULL, "isra"); + false, NULL, NULL, "isra"); current_function_decl = new_node->decl; push_cfun (DECL_STRUCT_FUNCTION (new_node->decl)); Index: tree-inline.c =================================================================== --- tree-inline.c (revision 182780) +++ tree-inline.c (working copy) @@ -2811,7 +2811,7 @@ declare_return_variable (copy_body_data /* We don't need to do anything for functions that don't return anything. */ - if (!result || VOID_TYPE_P (callee_type)) + if (VOID_TYPE_P (callee_type)) return NULL_TREE; /* If there was a return slot, then the return value is the @@ -5040,6 +5040,7 @@ update_clone_info (copy_body_data * id) If non-NULL ARGS_TO_SKIP determine function parameters to remove from new version. + If SKIP_RETURN is true, the new version will return void. If non-NULL BLOCK_TO_COPY determine what basic blocks to copy. If non_NULL NEW_ENTRY determine new entry BB of the clone. */ @@ -5047,7 +5048,8 @@ void tree_function_versioning (tree old_decl, tree new_decl, VEC(ipa_replace_map_p,gc)* tree_map, bool update_clones, bitmap args_to_skip, - bitmap blocks_to_copy, basic_block new_entry) + bool skip_return, bitmap blocks_to_copy, + basic_block new_entry) { struct cgraph_node *old_version_node; struct cgraph_node *new_version_node; @@ -5200,7 +5202,18 @@ tree_function_versioning (tree old_decl, /* Add local vars. */ add_local_variables (DECL_STRUCT_FUNCTION (old_decl), cfun, &id, false); - if (DECL_RESULT (old_decl) != NULL_TREE) + if (VOID_TYPE_P (TREE_TYPE (DECL_RESULT (old_decl)))) + ; + else if (skip_return) + { + DECL_RESULT (new_decl) + = build_decl (DECL_SOURCE_LOCATION (DECL_RESULT (old_decl)), + RESULT_DECL, NULL_TREE, void_type_node); + DECL_CONTEXT (DECL_RESULT (new_decl)) = new_decl; + cfun->returns_struct = 0; + cfun->returns_pcc_struct = 0; + } + else { tree old_name; DECL_RESULT (new_decl) = remap_decl (DECL_RESULT (old_decl), &id);