Browse Source

gcc: Add a preliminary buffer registration analysis pass.

* gcc-plugin/src/starpu.c (registration_in_bb_p, dominated_by_registration,
  ssa_name_aliases_global_or_parm_p, validate_task_invocations,
  warn_starpu_unregistered): New functions.
  (pass_lower_starpu): Change `name' field to "lower_starpu".
  (pass_warn_starpu_unregistered): New variable.
  (plugin_init): Register it.

* gcc-plugin/tests/warn-unregistered.c: New file.
* gcc-plugin/tests/Makefile.am (gcc_tests): Add it.

* gcc-plugin/tests/pointers.c (main): Make `x' static to avoid erroneous
  warning.
Ludovic Courtès 12 years ago
parent
commit
a022bd13cb

+ 1 - 0
.gitignore

@@ -288,3 +288,4 @@ starpu.log
 /gcc-plugin/tests/release
 /gcc-plugin/tests/opencl
 /gcc-plugin/tests/registered
+/gcc-plugin/tests/warn-unregistered

+ 247 - 5
gcc-plugin/src/starpu.c

@@ -32,6 +32,7 @@
 #include <tree.h>
 #include <tree-iterator.h>
 #include <langhooks.h>
+#include <flags.h>				  /* for `optimize' */
 
 #ifdef HAVE_C_FAMILY_C_COMMON_H
 # include <c-family/c-common.h>
@@ -3295,6 +3296,206 @@ validate_task_implementation (tree impl)
       }
 }
 
+/* Return true if there exists a `starpu_vector_data_register' call for VAR
+   before GSI in its basic block.  */
+
+static bool
+registration_in_bb_p (gimple_stmt_iterator gsi, tree var)
+{
+  gcc_assert (SSA_VAR_P (var));
+
+  tree register_fn_name;
+
+  register_fn_name = get_identifier ("starpu_vector_data_register");
+
+  local_define (bool, registration_function_p, (const_tree obj))
+  {
+    /* TODO: Compare against the real fndecl.  */
+    return (obj != NULL_TREE
+	    && TREE_CODE (obj) == FUNCTION_DECL
+	    && DECL_NAME (obj) == register_fn_name);
+  };
+
+  bool found;
+
+  for (found = false;
+       !gsi_end_p (gsi) && !found;
+       gsi_prev (&gsi))
+    {
+      gimple stmt;
+
+      stmt = gsi_stmt (gsi);
+      if (is_gimple_call (stmt))
+	{
+	  tree fn = gimple_call_fndecl (stmt);
+	  if (registration_function_p (fn))
+	    {
+	      tree arg = gimple_call_arg (stmt, 2);
+	      if (is_gimple_address (arg))
+	      	arg = TREE_OPERAND (arg, 0);
+
+	      if (((TREE_CODE (arg) == VAR_DECL
+		    || TREE_CODE (arg) == VAR_DECL)
+		   && refs_may_alias_p (arg, var))
+
+		  /* Both VAR and ARG should be SSA names, otherwise, if ARG
+		     is a VAR_DECL, `ptr_derefs_may_alias_p' will
+		     conservatively assume that they may alias.  */
+		  || (TREE_CODE (var) == SSA_NAME
+		      && TREE_CODE (arg) != VAR_DECL
+		      && ptr_derefs_may_alias_p (arg, var)))
+		{
+		  if (verbose_output_p)
+		    {
+		      var = TREE_CODE (var) == SSA_NAME ? SSA_NAME_VAR (var) : var;
+		      inform (gimple_location (stmt),
+			      "found registration of variable %qE",
+			      DECL_NAME (var));
+		    }
+		  found = true;
+		}
+	    }
+	}
+    }
+
+  return found;
+}
+
+/* Return true if BB is dominated by a registration of VAR.  */
+
+static bool
+dominated_by_registration (gimple_stmt_iterator gsi, tree var)
+{
+  /* Is there a registration call for VAR in GSI's basic block?  */
+  if (registration_in_bb_p (gsi, var))
+    return true;
+
+  edge e;
+  edge_iterator ei;
+  bool found = false;
+
+  /* If every incoming edge is dominated by a registration, then we're
+     fine.
+
+     FIXME: This triggers false positives when registration is done in a
+     loop, because there's always an edge through which no registration
+     happens--the edge corresponding to the case where the loop is not
+     entered.  */
+
+  FOR_EACH_EDGE (e, ei, gsi_bb (gsi)->preds)
+    {
+      if (!dominated_by_registration (gsi_last_bb (e->src), var))
+	return false;
+      else
+	found = true;
+    }
+
+  return found;
+}
+
+/* Return true if NAME aliases a global variable or a PARM_DECL.  Note that,
+   for the former, `ptr_deref_may_alias_global_p' is way too conservative,
+   hence this approach.  */
+
+static bool
+ssa_name_aliases_global_or_parm_p (const_tree name)
+{
+  gcc_assert (TREE_CODE (name) == SSA_NAME);
+
+  if (TREE_CODE (SSA_NAME_VAR (name)) == PARM_DECL)
+    return true;
+  else
+    {
+      gimple def_stmt;
+
+      def_stmt = SSA_NAME_DEF_STMT (name);
+      if (is_gimple_assign (def_stmt))
+	{
+	  tree rhs = gimple_assign_rhs1 (def_stmt);
+
+	  if (TREE_CODE (rhs) == VAR_DECL
+	      && (DECL_EXTERNAL (rhs) || TREE_STATIC (rhs)))
+	    return true;
+	}
+    }
+
+  return false;
+}
+
+
+/* Validate the arguments passed to tasks in FN's body.  */
+
+static void
+validate_task_invocations (tree fn)
+{
+  gcc_assert (TREE_CODE (fn) == FUNCTION_DECL);
+
+  const struct cgraph_node *cgraph;
+  const struct cgraph_edge *callee;
+
+  cgraph = cgraph_get_node (fn);
+
+  /* When a definition of IMPL is available, check its callees.  */
+  if (cgraph != NULL)
+    for (callee = cgraph->callees;
+	 callee != NULL;
+	 callee = callee->next_callee)
+      {
+	if (task_p (callee->callee->decl))
+	  {
+	    unsigned i;
+	    gimple call_stmt = callee->call_stmt;
+
+	    for (i = 0; i < gimple_call_num_args (call_stmt); i++)
+	      {
+		tree arg = gimple_call_arg (call_stmt, i);
+
+		if (TREE_CODE (arg) == ADDR_EXPR
+		    && TREE_CODE (TREE_OPERAND (arg, 0)) == VAR_DECL
+		    && (TREE_CODE (TREE_TYPE (TREE_OPERAND (arg, 0)))
+			== ARRAY_TYPE))
+		  /* This is a "pointer-to-array" of a variable, so what we
+		     really care about is the variable itself.  */
+		  arg = TREE_OPERAND (arg, 0);
+
+		if ((POINTER_TYPE_P (TREE_TYPE (arg))
+		     || (TREE_CODE (TREE_TYPE (arg)) == ARRAY_TYPE))
+		    && ((TREE_CODE (arg) == VAR_DECL
+			 && !TREE_STATIC (arg)
+			 && !DECL_EXTERNAL (arg)
+			 && !TREE_NO_WARNING (arg))
+			|| (TREE_CODE (arg) == SSA_NAME
+			    && !ssa_name_aliases_global_or_parm_p (arg))))
+		  {
+		    if (!dominated_by_registration (gsi_for_stmt (call_stmt),
+						    arg))
+		      {
+			if (TREE_CODE (arg) == SSA_NAME)
+			  {
+			    tree var = SSA_NAME_VAR (arg);
+			    if (DECL_NAME (var) != NULL)
+			      arg = var;
+
+			    /* TODO: Check whether we can get the original
+			       variable name via ARG's DEF_STMT.  */
+			  }
+
+			if (TREE_CODE (arg) == VAR_DECL
+			    && DECL_NAME (arg) != NULL_TREE)
+			  warning_at (gimple_location (call_stmt), 0,
+				      "variable %qE may be used unregistered",
+				      DECL_NAME (arg));
+			else
+			  warning_at (gimple_location (call_stmt), 0,
+				      "argument %i may be used unregistered",
+				      i);
+		      }
+		  }
+	      }
+	  }
+      }
+}
+
 static unsigned int
 lower_starpu (void)
 {
@@ -3384,21 +3585,44 @@ lower_starpu (void)
 	    inform (gimple_location (callee->call_stmt),
 		    "%qE calls task %qE",
 		    DECL_NAME (fndecl), DECL_NAME (callee_decl));
-
-	  /* TODO: Insert analysis to check whether the pointer arguments
-	     need to be registered.  */
 	}
     }
 
   return 0;
 }
 
+/* A pass to warn about possibly unregistered task arguments.  */
+
+static unsigned int
+warn_starpu_unregistered (void)
+{
+  tree fndecl;
+
+  fndecl = current_function_decl;
+  gcc_assert (TREE_CODE (fndecl) == FUNCTION_DECL);
+
+  if (!task_p (fndecl))
+    validate_task_invocations (fndecl);
+
+  return 0;
+}
+
 static struct opt_pass pass_lower_starpu =
   {
     designated_field_init (type, GIMPLE_PASS),
-    designated_field_init (name, "pass_lower_starpu"),
+    designated_field_init (name, "lower_starpu"),
+    designated_field_init (gate, NULL),
+    designated_field_init (execute, lower_starpu),
+
+    /* The rest is zeroed.  */
+  };
+
+static struct opt_pass pass_warn_starpu_unregistered =
+  {
+    designated_field_init (type, GIMPLE_PASS),
+    designated_field_init (name, "warn_starpu_unregistered"),
     designated_field_init (gate, NULL),
-    designated_field_init (execute, lower_starpu)
+    designated_field_init (execute, warn_starpu_unregistered),
 
     /* The rest is zeroed.  */
   };
@@ -3480,6 +3704,24 @@ plugin_init (struct plugin_name_args *plugin_info,
   register_callback (plugin_name, PLUGIN_PASS_MANAGER_SETUP,
 		     NULL, &pass_info);
 
+  struct register_pass_info pass_info2 =
+    {
+      designated_field_init (pass, &pass_warn_starpu_unregistered),
+      designated_field_init (reference_pass_name, "ssa"),
+      designated_field_init (ref_pass_instance_number, 1),
+      designated_field_init (pos_op, PASS_POS_INSERT_AFTER)
+    };
+
+  if (optimize)
+    /* Using `TODO_rebuild_alias' allows us to have more accurate aliasing
+       info.  However, `TODO_rebuild_alias' cannot be used when optimizations
+       are turned off.  See <http://gcc.gnu.org/ml/gcc/2012-10/msg00104.html>
+       for details.  */
+    pass_warn_starpu_unregistered.todo_flags_start = TODO_rebuild_alias;
+
+  register_callback (plugin_name, PLUGIN_PASS_MANAGER_SETUP,
+		     NULL, &pass_info2);
+
   include_dir = getenv ("STARPU_GCC_INCLUDE_DIR");
   opencl_include_dirs = tree_cons (NULL_TREE, build_string (1, "."),
 				   NULL_TREE);

+ 1 - 0
gcc-plugin/tests/Makefile.am

@@ -29,6 +29,7 @@ gcc_tests =					\
   release-errors.c				\
   unregister.c					\
   unregister-errors.c				\
+  warn-unregistered.c				\
   task-errors.c					\
   scalar-tasks.c				\
   pointer-tasks.c				\

+ 1 - 1
gcc-plugin/tests/pointers.c

@@ -71,7 +71,7 @@ main (int argc, char *argv[])
 #pragma starpu initialize
 
   static const unsigned char z = 0x77;
-  int x[] = { 42 };
+  static int x[] = { 42 };
   short *y;
 
   y = malloc (sizeof *y);

+ 211 - 0
gcc-plugin/tests/warn-unregistered.c

@@ -0,0 +1,211 @@
+/* GCC-StarPU
+   Copyright (C) 2012 Institut National de Recherche en Informatique et Automatique
+
+   GCC-StarPU is free software: you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation, either version 3 of the License, or
+   (at your option) any later version.
+
+   GCC-StarPU is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with GCC-StarPU.  If not, see <http://www.gnu.org/licenses/>.  */
+
+/* (instructions compile) */
+
+/* Make sure warnings get raised when pointer variables are likely never
+   registered.  */
+
+extern void my_task (double *x, double *y)
+  __attribute__ ((task));
+
+extern double *global1;
+double global2[123];
+
+void
+parm_decl_no_warn (double *parm1, double *parm2)
+{
+  my_task (parm1, parm2);	  /* no warning, because these are parameters
+				     so we cannot tell anything */
+}
+
+void
+global_decl_no_warn (void)
+{
+  my_task (global1, global2);
+}
+
+void
+two_unregistered_pointers (void)
+{
+  double *p, *q;
+
+  p = malloc (12 * sizeof *p);
+  q = malloc (23 * sizeof *q);
+
+  my_task (p, q); /* (warning "p.* used unregistered") *//* (warning "q.* used unregistered") */
+}
+
+void
+one_unregistered_pointer (void)
+{
+  double *p, *q;
+
+  p = malloc (12 * sizeof *p);
+  q = malloc (23 * sizeof *q);
+
+#pragma starpu register p 12
+  my_task (p, q);		      /* (if optimizing?
+					     (warning "q.* used unregistered")) */
+}
+
+void
+another_unregistered_pointer (void)
+{
+  double X[] = { 1, 2, 3, 4 };
+  double *Y;
+
+  Y = malloc (123 * sizeof *Y);
+  if (Y == NULL)
+    return;
+  else
+    {
+      extern void frob (double *);
+      frob (Y);
+    }
+  X[0] = 42;
+
+#pragma starpu register Y 123
+  my_task (X, Y);		      /* (warning "X.* used unregistered") */
+}
+
+void
+zero_unregistered_pointers (void)
+{
+  double *p, *q;
+
+  p = malloc (12 * sizeof *p);
+  q = malloc (23 * sizeof *q);
+
+#pragma starpu register p 12
+#pragma starpu register q 23
+  my_task (p, q);				  /* no warning */
+}
+
+void
+two_pointers_unregistered_before_call (void)
+{
+  double *p, *q;
+
+  p = malloc (12 * sizeof *p);
+  q = malloc (23 * sizeof *q);
+
+  my_task (p, q); /* (warning "p.* used unregistered") *//* (warning "q.* used unregistered") */
+
+#pragma starpu register p 12
+#pragma starpu register q 23
+}
+
+void
+one_unregistered_array (void)
+{
+  double PPP[12], QQQ[23];
+
+#pragma starpu register PPP	      /* (warning "on-stack .* unsafe") */
+  my_task (PPP, QQQ);		      /* (warning "QQQ.* used unregistered") */
+}
+
+void
+not_the_ones_registered (void)
+{
+  double a[12], b[23], p[12], q[23];
+
+#pragma starpu register a		 /* (warning "on-stack .* unsafe") */
+#pragma starpu register b		 /* (warning "on-stack .* unsafe") */
+
+  my_task (p, q); /* (warning "p.* used unregistered") */ /* (warning "q.* used unregistered") */
+}
+
+void
+registered_pointers_with_aliases (void)
+{
+  double *a, *b, *p, *q;
+
+  a = malloc (123 * sizeof *a);
+  b = malloc (234 * sizeof *b);
+
+#pragma starpu register a 123
+#pragma starpu register b 234
+  p = a;
+  q = b;
+  my_task (p, q);				  /* no warning */
+}
+
+void
+one_unregistered_array_attrs (void)
+{
+  double p[12];
+  double q[23] __attribute__ ((heap_allocated, registered));
+
+  my_task (p, q);		      /* (warning "p.* used unregistered") */
+}
+
+void
+unregistered_on_one_path (int x)
+{
+  double p[12], q[34];
+
+  if (x > 42)
+    {
+#pragma starpu register p	      /* (warning "on-stack .* unsafe") */
+    }
+
+#pragma starpu register q	      /* (warning "on-stack .* unsafe") */
+
+  my_task (p, q);		      /* (warning "p.* used unregistered") */
+}
+
+void
+registered_via_two_paths (int x)
+{
+  double p[12], q[34];
+
+  if (x > 42)
+    {
+#pragma starpu register p	      /* (warning "on-stack .* unsafe") */
+    }
+  else
+    {
+#pragma starpu register p	      /* (warning "on-stack .* unsafe") */
+    }
+
+#pragma starpu register q	      /* (warning "on-stack .* unsafe") */
+
+  my_task (p, q);				  /* no warning */
+}
+
+#if 0
+
+/* FIXME: This case currently triggers a false positives.  */
+
+void
+registered_and_used_in_loop (void)
+{
+  int i;
+  double *p[123];
+  static double q[234];
+
+  for (i = 0; i < 123; i++)
+    {
+      p[i] = malloc (123 * sizeof *p[i]);
+#pragma starpu register p[i] 123
+    }
+
+  for (i = 0; i < 123; i++)
+    my_task (p[i], q);
+}
+
+#endif