diff mbox

Fix ICEs with functions returning variable-sized array

Message ID 201201102027.15802.ebotcazou@adacore.com
State New
Headers show

Commit Message

Eric Botcazou Jan. 10, 2012, 7:27 p.m. UTC
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  <ebotcazou@adacore.com>

	* 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  <ebotcazou@adacore.com>

	* gnat.dg/opt23.ad[sb]: New test.
	* gnat.dg/opt23_pkg.ad[sb]: New helper.
	* gnat.dg/opt24.ad[sb]: New test.

Comments

Richard Biener Jan. 11, 2012, 9:05 a.m. UTC | #1
On Tue, Jan 10, 2012 at 8:27 PM, Eric Botcazou <ebotcazou@adacore.com> wrote:
> 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?

Ok.

Thanks,
Richard.

>
> 2012-01-10  Eric Botcazou  <ebotcazou@adacore.com>
>
>        * 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  <ebotcazou@adacore.com>
>
>        * gnat.dg/opt23.ad[sb]: New test.
>        * gnat.dg/opt23_pkg.ad[sb]: New helper.
>        * gnat.dg/opt24.ad[sb]: New test.
>
>
> --
> Eric Botcazou
diff mbox

Patch

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);