public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Eric Botcazou <ebotcazou@adacore.com>
To: gcc-patches@gcc.gnu.org
Subject: [patch] Fix ICE on function [not] returning variable size
Date: Mon, 01 Jun 2015 10:45:00 -0000	[thread overview]
Message-ID: <2953955.A7ElB1Yu05@polaris> (raw)

[-- Attachment #1: Type: text/plain, Size: 1386 bytes --]

Hi,

this fixes an ICE on a function returning a variable-sized record type but 
discovered to be no-return by the optimizer.  In this case, the LHS of the 
GIMPLE call statement is removed so the RTL expander attempts to allocate a 
temporary and fails:

eric@polaris:~/build/gcc/native> gcc/gnat1 -quiet varsize_return2.ads -O
+===========================GNAT BUG DETECTED==============================+
| 6.0.0 20150531 (experimental) [trunk revision 223897] (x86_64-suse-linux) 
GCC error:|
| in assign_stack_temp_for_type, at function.c:793        

because it too cannot create temporaries of variable size.

The attached patch simply preserves the LHS throughout the GIMPLE pipeline.
It also simplifies the relevant test in gimplify_modify_expr_rhs, which was 
overly broad.

Tested on x86_64-suse-linux, OK for the mainline?


2015-06-01  Eric Botcazou  <ebotcazou@adacore.com>

	* gimplify.c (gimplify_modify_expr_rhs): Use simple test on the size.
	* cgraph.c (cgraph_redirect_edge_call_stmt_to_callee): Do not remove
	the LHS of a no-return call if its type has variable size.
	* tree-cfgcleanup.c (fixup_noreturn_call): Likewise.
	* tree-cfg.c (verify_gimple_call): Accept these no-return calls.


2015-06-01  Eric Botcazou  <ebotcazou@adacore.com>

	* gnat.dg/specs/varsize_return2.ads: New test.
	* gnat.dg/specs/varsize_return2_pkg.ad[sb]: New helper.


-- 
Eric Botcazou

[-- Attachment #2: p.diff --]
[-- Type: text/x-patch, Size: 3916 bytes --]

Index: cgraph.c
===================================================================
--- cgraph.c	(revision 223897)
+++ cgraph.c	(working copy)
@@ -1449,8 +1449,10 @@ cgraph_edge::redirect_call_stmt_to_calle
       update_stmt_fn (DECL_STRUCT_FUNCTION (e->caller->decl), new_stmt);
     }
 
-  /* If the call becomes noreturn, remove the lhs.  */
-  if (lhs && (gimple_call_flags (new_stmt) & ECF_NORETURN))
+  /* If the call becomes noreturn, remove the LHS if possible.  */
+  if (lhs
+      && (gimple_call_flags (new_stmt) & ECF_NORETURN)
+      && TREE_CODE (TYPE_SIZE_UNIT (TREE_TYPE (lhs))) == INTEGER_CST)
     {
       if (TREE_CODE (lhs) == SSA_NAME)
 	{
Index: gimplify.c
===================================================================
--- gimplify.c	(revision 223897)
+++ gimplify.c	(working copy)
@@ -4341,7 +4341,8 @@ gimplify_modify_expr_rhs (tree *expr_p,
 		/* It's OK to use the target directly if it's being
 		   initialized. */
 		use_target = true;
-	      else if (variably_modified_type_p (TREE_TYPE (*to_p), NULL_TREE))
+	      else if (TREE_CODE (TYPE_SIZE_UNIT (TREE_TYPE (*to_p)))
+		       != INTEGER_CST)
 		/* Always use the target and thus RSO for variable-sized types.
 		   GIMPLE cannot deal with a variable-sized assignment
 		   embedded in a call statement.  */
Index: tree-cfgcleanup.c
===================================================================
--- tree-cfgcleanup.c	(revision 223897)
+++ tree-cfgcleanup.c	(working copy)
@@ -612,9 +612,11 @@ fixup_noreturn_call (gimple stmt)
 	}
     }
 
-  /* If there is an LHS, remove it.  */
+  /* If there is an LHS, remove it, but only if its type has fixed size.
+     The LHS will need to be recreated during RTL expansion and creating
+     temporaries of variable-sized types is not supported.  */
   tree lhs = gimple_call_lhs (stmt);
-  if (lhs)
+  if (lhs && TREE_CODE (TYPE_SIZE_UNIT (TREE_TYPE (lhs))) == INTEGER_CST)
     {
       gimple_call_set_lhs (stmt, NULL_TREE);
 
Index: tree-cfg.c
===================================================================
--- tree-cfg.c	(revision 223897)
+++ tree-cfg.c	(working copy)
@@ -3391,17 +3391,19 @@ verify_gimple_call (gcall *stmt)
        return true;
      }
 
-  if (gimple_call_lhs (stmt)
-      && (!is_gimple_lvalue (gimple_call_lhs (stmt))
-	  || verify_types_in_gimple_reference (gimple_call_lhs (stmt), true)))
+  tree lhs = gimple_call_lhs (stmt);
+  if (lhs
+      && (!is_gimple_lvalue (lhs)
+	  || verify_types_in_gimple_reference (lhs, true)))
     {
       error ("invalid LHS in gimple call");
       return true;
     }
 
-  if (gimple_call_ctrl_altering_p (stmt)
-      && gimple_call_lhs (stmt)
-      && gimple_call_noreturn_p (stmt))
+  if (lhs
+      && gimple_call_ctrl_altering_p (stmt)
+      && gimple_call_noreturn_p (stmt)
+      && TREE_CODE (TYPE_SIZE_UNIT (TREE_TYPE (lhs))) == INTEGER_CST)
     {
       error ("LHS in noreturn call");
       return true;
@@ -3409,19 +3411,18 @@ verify_gimple_call (gcall *stmt)
 
   fntype = gimple_call_fntype (stmt);
   if (fntype
-      && gimple_call_lhs (stmt)
-      && !useless_type_conversion_p (TREE_TYPE (gimple_call_lhs (stmt)),
-				     TREE_TYPE (fntype))
+      && lhs
+      && !useless_type_conversion_p (TREE_TYPE (lhs), TREE_TYPE (fntype))
       /* ???  At least C++ misses conversions at assignments from
 	 void * call results.
 	 ???  Java is completely off.  Especially with functions
 	 returning java.lang.Object.
 	 For now simply allow arbitrary pointer type conversions.  */
-      && !(POINTER_TYPE_P (TREE_TYPE (gimple_call_lhs (stmt)))
+      && !(POINTER_TYPE_P (TREE_TYPE (lhs))
 	   && POINTER_TYPE_P (TREE_TYPE (fntype))))
     {
       error ("invalid conversion in gimple call");
-      debug_generic_stmt (TREE_TYPE (gimple_call_lhs (stmt)));
+      debug_generic_stmt (TREE_TYPE (lhs));
       debug_generic_stmt (TREE_TYPE (fntype));
       return true;
     }

[-- Attachment #3: varsize_return2.ads --]
[-- Type: text/x-adasrc, Size: 213 bytes --]

-- { dg-do compile }
-- { dg-options "-O" }

with Varsize_Return2_Pkg; use Varsize_Return2_Pkg;

package Varsize_Return2 is

   package My_G is new G (0);

   Result : constant T := My_G.Get;

end Varsize_Return2;

[-- Attachment #4: varsize_return2_pkg.adb --]
[-- Type: text/x-adasrc, Size: 310 bytes --]

package body Varsize_Return2_Pkg is

   function Len return Positive is
   begin
      return 4;
   end;

   package body G is

      function Get return Small_T is
      begin
         raise Program_Error;
         return Get;
      end;

   end G;

end Varsize_Return2_Pkg;\r

[-- Attachment #5: varsize_return2_pkg.ads --]
[-- Type: text/x-adasrc, Size: 341 bytes --]

-- { dg-excess-errors "no code generated" }

package Varsize_Return2_Pkg is

   type T (D: Positive) is record
      Data: String (1 .. D);
   end record;

   function Len return Positive;

   generic
      I : Integer;
   package G is

      subtype Small_T is T(Len);
      function Get return Small_T;

   end G;

end Varsize_Return2_Pkg;

             reply	other threads:[~2015-06-01 10:45 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-06-01 10:45 Eric Botcazou [this message]
2015-06-01 11:19 ` Richard Biener
2015-06-01 21:10   ` Eric Botcazou
2015-06-02  8:30     ` Richard Biener
2015-06-02  9:30       ` Eric Botcazou

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=2953955.A7ElB1Yu05@polaris \
    --to=ebotcazou@adacore.com \
    --cc=gcc-patches@gcc.gnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).