Patchwork [PR,60647] Check that actual argument types match those of formal parameters before IPA-SRA

login
register
mail settings
Submitter Martin Jambor
Date March 28, 2014, 4:35 p.m.
Message ID <20140328163511.GU19304@virgil.suse>
Download mbox | patch
Permalink /patch/334784/
State New
Headers show

Comments

Martin Jambor - March 28, 2014, 4:35 p.m.
Hi,

after much confusion on my part, this is the proper fix for PR 60647.
IPA-SRA can get confused a lot when a formal parameter is a pointer
but the corresponding actual argument is not.  So this patch adds a
check that their types pass useless_type_conversion_p check.

Bootstrapped and tested on x86_64-linux.  OK for trunk?

Thanks,

Martin


2014-03-28  Martin Jambor  <mjambor@suse.cz>

	PR middle-end/60647
	* tree-sra.c (callsite_has_enough_arguments_p): Renamed to
	callsite_arguments_match_p.  Updated all callers.  Also check types of
	corresponding formal parameters and actual arguments.
	(not_all_callers_have_enough_arguments_p) Renamed to
	some_callers_have_mismatched_arguments_p.

testsuite/
	* gcc.dg/pr60647-1.c: New test.
	* gcc.dg/pr60647-2.c: Likewise.
Jakub Jelinek - March 28, 2014, 5:20 p.m.
On Fri, Mar 28, 2014 at 05:35:12PM +0100, Martin Jambor wrote:
> after much confusion on my part, this is the proper fix for PR 60647.
> IPA-SRA can get confused a lot when a formal parameter is a pointer
> but the corresponding actual argument is not.  So this patch adds a
> check that their types pass useless_type_conversion_p check.
> 
> Bootstrapped and tested on x86_64-linux.  OK for trunk?

Ok, thanks.

> 2014-03-28  Martin Jambor  <mjambor@suse.cz>
> 
> 	PR middle-end/60647
> 	* tree-sra.c (callsite_has_enough_arguments_p): Renamed to
> 	callsite_arguments_match_p.  Updated all callers.  Also check types of
> 	corresponding formal parameters and actual arguments.
> 	(not_all_callers_have_enough_arguments_p) Renamed to
> 	some_callers_have_mismatched_arguments_p.
> 
> testsuite/
> 	* gcc.dg/pr60647-1.c: New test.
> 	* gcc.dg/pr60647-2.c: Likewise.

	Jakub

Patch

diff --git a/gcc/testsuite/gcc.dg/pr60647-1.c b/gcc/testsuite/gcc.dg/pr60647-1.c
new file mode 100644
index 0000000..73ea856
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr60647-1.c
@@ -0,0 +1,26 @@ 
+/* { dg-do compile } */
+/* { dg-options "-O2" } */
+
+struct _wincore
+{
+  int y;
+  int width;
+};
+int a;
+static fn1 (dpy, winInfo) struct _XDisplay *dpy;
+struct _wincore *winInfo;
+{
+  a = winInfo->width;
+  fn2 ();
+}
+
+static fn3 (dpy, winInfo, visrgn) struct _XDisplay *dpy;
+{
+  int b = fn1 (0, winInfo);
+  fn4 (0, 0, visrgn);
+}
+
+fn5 (event) struct _XEvent *event;
+{
+  fn3 (0, 0, 0);
+}
diff --git a/gcc/testsuite/gcc.dg/pr60647-2.c b/gcc/testsuite/gcc.dg/pr60647-2.c
new file mode 100644
index 0000000..ddeb117
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr60647-2.c
@@ -0,0 +1,26 @@ 
+/* { dg-do compile } */
+/* { dg-options "-O2" } */
+
+struct _wincore
+{
+  int width, height;
+};
+
+static void
+foo (void *dpy, struct _wincore *winInfo, int offset)
+{
+  fn1 (winInfo->height);
+}
+
+static void
+bar (void *dpy, int winInfo, int *visrgn)
+{
+  ((void (*) (void *, int, int)) foo) ((void *) 0, winInfo, 0);  /* { dg-warning "function called through a non-compatible type" } */
+  fn2 (0, 0, visrgn);
+}
+
+void
+baz (void *dpy, int win, int prop)
+{
+  bar ((void *) 0, 0, (int *) 0);
+}
diff --git a/gcc/tree-sra.c b/gcc/tree-sra.c
index 284d544..ffef13d 100644
--- a/gcc/tree-sra.c
+++ b/gcc/tree-sra.c
@@ -1234,12 +1234,26 @@  asm_visit_addr (gimple, tree op, tree, void *)
 }
 
 /* Return true iff callsite CALL has at least as many actual arguments as there
-   are formal parameters of the function currently processed by IPA-SRA.  */
+   are formal parameters of the function currently processed by IPA-SRA and
+   that their types match.  */
 
 static inline bool
-callsite_has_enough_arguments_p (gimple call)
+callsite_arguments_match_p (gimple call)
 {
-  return gimple_call_num_args (call) >= (unsigned) func_param_count;
+  if (gimple_call_num_args (call) < (unsigned) func_param_count)
+    return false;
+
+  tree parm;
+  int i;
+  for (parm = DECL_ARGUMENTS (current_function_decl), i = 0;
+       parm;
+       parm = DECL_CHAIN (parm), i++)
+    {
+      tree arg = gimple_call_arg (call, i);
+      if (!useless_type_conversion_p (TREE_TYPE (parm), TREE_TYPE (arg)))
+	return false;
+    }
+  return true;
 }
 
 /* Scan function and look for interesting expressions and create access
@@ -1294,7 +1308,7 @@  scan_function (void)
 		      if (recursive_call_p (current_function_decl, dest))
 			{
 			  encountered_recursive_call = true;
-			  if (!callsite_has_enough_arguments_p (stmt))
+			  if (!callsite_arguments_match_p (stmt))
 			    encountered_unchangable_recursive_call = true;
 			}
 		    }
@@ -4750,16 +4764,17 @@  sra_ipa_reset_debug_stmts (ipa_parm_adjustment_vec adjustments)
     }
 }
 
-/* Return false iff all callers have at least as many actual arguments as there
-   are formal parameters in the current function.  */
+/* Return false if all callers have at least as many actual arguments as there
+   are formal parameters in the current function and that their types
+   match.  */
 
 static bool
-not_all_callers_have_enough_arguments_p (struct cgraph_node *node,
-					 void *data ATTRIBUTE_UNUSED)
+some_callers_have_mismatched_arguments_p (struct cgraph_node *node,
+					  void *data ATTRIBUTE_UNUSED)
 {
   struct cgraph_edge *cs;
   for (cs = node->callers; cs; cs = cs->next_caller)
-    if (!callsite_has_enough_arguments_p (cs->call_stmt))
+    if (!callsite_arguments_match_p (cs->call_stmt))
       return true;
 
   return false;
@@ -4970,12 +4985,13 @@  ipa_early_sra (void)
       goto simple_out;
     }
 
-  if (cgraph_for_node_and_aliases (node, not_all_callers_have_enough_arguments_p,
+  if (cgraph_for_node_and_aliases (node,
+				   some_callers_have_mismatched_arguments_p,
 				   NULL, true))
     {
       if (dump_file)
 	fprintf (dump_file, "There are callers with insufficient number of "
-		 "arguments.\n");
+		 "arguments or arguments with type mismatches.\n");
       goto simple_out;
     }