This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [C++ PATCH] Fix up DECL_ARG_TYPE (PR c++/36631)


On Wed, Nov 19, 2008 at 09:06:37AM +0100, Jakub Jelinek wrote:
> On Tue, Nov 18, 2008 at 07:00:29PM -0500, Jason Merrill wrote:
> > I note that the code in gimplify_call_expr is the only place in the  
> > compiler that checks the DECL_ARG_TYPE of a called function; everywhere  
> > else only uses it for the parameters of the function currently being  
> > compiled.  This is why this mismatch has not been a problem in the past.  
> >  Perhaps gimplify_call_expr shouldn't be relying on it?
> 
> Using just TREE_TYPE of the PARM_DECL isn't any better with the C++ FE,
> that's also incorrect until the function has been through
> cp_genericize.  And it would be wrong for types smaller than int
> which are promoted to int.
> 
> As I said earlier, I can easily move this checking from gimplify_call_expr
> until gimple lowering, at which point the C++ FE should be through
> cp_genericize with most of the functions (exceptions are late generated
> functions, e.g. during OpenMP lowering, but those can handle DECL_ARG_TYPE
> explicitly in the langhooks).  This would make DECL_ARG_TYPE correct
> for all functions that can be inlined (where bodies exist), but still
> DECL_ARG_TYPE (and TREE_TYPE) of PARM_DECLs would be wrong for DECL_EXTERNAL
> functions.

Here is a patch which does that.  Bootstrapped/regtested on x86_64-linux,
fixes the testcase.  Still for DECL_EXTERNAL routines DECL_ARG_TYPE and
TREE_TYPE of PARM_DECLs will be incorrect, which doesn't affect inlining (as
we don't have a body), but is still wrong.

2008-11-19  Jakub Jelinek  <jakub@redhat.com>

	PR c++/36631
	* gimplify.c (gimplify_call_expr): Defer most of the cannot inline
	checking until GIMPLE lowering.
	* gimple-low.c (check_call_args): New function.
	(lower_stmt) <case GIMPLE_CALL>: Call it.

	* g++.dg/template/call5.C: New test.
	
--- gcc/gimplify.c.jj	2008-11-18 19:24:09.000000000 +0100
+++ gcc/gimplify.c	2008-11-19 11:23:06.000000000 +0100
@@ -2352,56 +2352,18 @@ gimplify_call_expr (tree *expr_p, gimple
   else if (POINTER_TYPE_P (TREE_TYPE (CALL_EXPR_FN (*expr_p))))
     parms = TYPE_ARG_TYPES (TREE_TYPE (TREE_TYPE (CALL_EXPR_FN (*expr_p))));
 
-  /* Verify if the type of the argument matches that of the function
-     declaration.  If we cannot verify this or there is a mismatch,
-     mark the call expression so it doesn't get inlined later.  */
   if (fndecl && DECL_ARGUMENTS (fndecl))
-    {
-      for (i = 0, p = DECL_ARGUMENTS (fndecl);
-	   i < nargs;
-	   i++, p = TREE_CHAIN (p))
-	{
-	  /* We cannot distinguish a varargs function from the case
-	     of excess parameters, still deferring the inlining decision
-	     to the callee is possible.  */
-	  if (!p)
-	    break;
-	  if (p == error_mark_node
-	      || CALL_EXPR_ARG (*expr_p, i) == error_mark_node
-	      || !fold_convertible_p (DECL_ARG_TYPE (p),
-				      CALL_EXPR_ARG (*expr_p, i)))
-	    {
-	      CALL_CANNOT_INLINE_P (*expr_p) = 1;
-	      break;
-	    }
-	}
-    }
+    p = DECL_ARGUMENTS (fndecl);
   else if (parms)
-    {
-      for (i = 0, p = parms; i < nargs; i++, p = TREE_CHAIN (p))
-	{
-	  /* If this is a varargs function defer inlining decision
-	     to callee.  */
-	  if (!p)
-	    break;
-	  if (TREE_VALUE (p) == error_mark_node
-	      || CALL_EXPR_ARG (*expr_p, i) == error_mark_node
-	      || TREE_CODE (TREE_VALUE (p)) == VOID_TYPE
-	      || !fold_convertible_p (TREE_VALUE (p),
-				      CALL_EXPR_ARG (*expr_p, i)))
-	    {
-	      CALL_CANNOT_INLINE_P (*expr_p) = 1;
-	      break;
-	    }
-	}
-    }
+    p = parms;
   else
     {
       if (nargs != 0)
 	CALL_CANNOT_INLINE_P (*expr_p) = 1;
-      i = 0;
       p = NULL_TREE;
     }
+  for (i = 0; i < nargs && p; i++, p = TREE_CHAIN (p))
+    ;
 
   /* If the last argument is __builtin_va_arg_pack () and it is not
      passed as a named argument, decrease the number of CALL_EXPR
--- gcc/gimple-low.c.jj	2008-10-23 13:21:39.000000000 +0200
+++ gcc/gimple-low.c	2008-11-19 10:19:28.000000000 +0100
@@ -1,6 +1,7 @@
 /* GIMPLE lowering pass.  Converts High GIMPLE into Low GIMPLE.
 
-   Copyright (C) 2003, 2004, 2005, 2006, 2007, 2008 Free Software Foundation, Inc.
+   Copyright (C) 2003, 2004, 2005, 2006, 2007, 2008
+   Free Software Foundation, Inc.
 
 This file is part of GCC.
 
@@ -218,6 +219,80 @@ struct gimple_opt_pass pass_lower_cf = 
 };
 
 
+/* Verify if the type of the argument matches that of the function
+   declaration.  If we cannot verify this or there is a mismatch,
+   mark the call expression so it doesn't get inlined later.  */
+
+static void
+check_call_args (gimple stmt)
+{
+  tree fndecl, parms, p;
+  unsigned int i, nargs;
+
+  if (gimple_call_cannot_inline_p (stmt))
+    return;
+
+  nargs = gimple_call_num_args (stmt);
+
+  /* Get argument types for verification.  */
+  fndecl = gimple_call_fndecl (stmt);
+  parms = NULL_TREE;
+  if (fndecl)
+    parms = TYPE_ARG_TYPES (TREE_TYPE (fndecl));
+  else if (POINTER_TYPE_P (TREE_TYPE (gimple_call_fn (stmt))))
+    parms = TYPE_ARG_TYPES (TREE_TYPE (TREE_TYPE (gimple_call_fn (stmt))));
+
+  /* Verify if the type of the argument matches that of the function
+     declaration.  If we cannot verify this or there is a mismatch,
+     mark the call expression so it doesn't get inlined later.  */
+  if (fndecl && DECL_ARGUMENTS (fndecl))
+    {
+      for (i = 0, p = DECL_ARGUMENTS (fndecl);
+	   i < nargs;
+	   i++, p = TREE_CHAIN (p))
+	{
+	  /* We cannot distinguish a varargs function from the case
+	     of excess parameters, still deferring the inlining decision
+	     to the callee is possible.  */
+	  if (!p)
+	    break;
+	  if (p == error_mark_node
+	      || gimple_call_arg (stmt, i) == error_mark_node
+	      || !fold_convertible_p (DECL_ARG_TYPE (p),
+				      gimple_call_arg (stmt, i)))
+	    {
+	      gimple_call_set_cannot_inline (stmt, true);
+	      break;
+	    }
+	}
+    }
+  else if (parms)
+    {
+      for (i = 0, p = parms; i < nargs; i++, p = TREE_CHAIN (p))
+	{
+	  /* If this is a varargs function defer inlining decision
+	     to callee.  */
+	  if (!p)
+	    break;
+	  if (TREE_VALUE (p) == error_mark_node
+	      || gimple_call_arg (stmt, i) == error_mark_node
+	      || TREE_CODE (TREE_VALUE (p)) == VOID_TYPE
+	      || !fold_convertible_p (TREE_VALUE (p),
+				      gimple_call_arg (stmt, i)))
+	    {
+	      gimple_call_set_cannot_inline (stmt, true);
+	      break;
+	    }
+	}
+    }
+  else
+    {
+      if (nargs != 0)
+	gimple_call_set_cannot_inline (stmt, true);
+    }
+}
+
+
 /* Lower sequence SEQ.  Unlike gimplification the statements are not relowered
    when they are changed -- if this has to be done, the lowering routine must
    do it explicitly.  DATA is passed through the recursion.  */
@@ -320,6 +395,7 @@ lower_stmt (gimple_stmt_iterator *gsi, s
 	    lower_builtin_setjmp (gsi);
 	    return;
 	  }
+	check_call_args (stmt);
       }
       break;
 
--- gcc/testsuite/g++.dg/template/call5.C.jj	2008-11-19 11:44:13.000000000 +0100
+++ gcc/testsuite/g++.dg/template/call5.C	2008-11-19 11:38:27.000000000 +0100
@@ -0,0 +1,17 @@
+// PR c++/36631
+// { dg-options "-O0" }
+
+template <typename T> struct B
+{
+  struct C
+  {
+    __attribute__ ((always_inline)) C (C const &c) {}
+  };
+  void __attribute__ ((always_inline)) g (C c) {}
+};
+
+void
+trigger (B <int> b, B <int>::C c)
+{
+  b.g (c);
+}


	Jakub


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]