invoke.texi (-Wsuggest-attribute=const, [...]): Document.

2010-04-27  Manuel López-Ibáñez  <manu@gcc.gnu.org>
	    Jan Hubicka <hubicka@ucw.cz>

	* doc/invoke.texi (-Wsuggest-attribute=const,
	-Wsuggest-attribute=pure): Document.
	* ipa-pure-const.c: Include toplev.h, intl.h and opts.h.
	(function_always_visible_to_compiler_p,
	suggest_attribute, warn_function_pure, warn_function_const):
	New functions.
	(check_call): Improve debug info.
	(analyze_function): Do not check availability.
	(add_new_function): Check availability.
	(propagate): Output warnings.
	(skip_function_for_local_pure_const): New function.
	(local_pure_const): Use it; output warnings.
	* common.opt (Wsuggest-attribute=const,
	Wsuggest-attribute=pure): New.

testsuite/
	* gcc.dg/pure-2.c: New testcase.
	* gcc.dg/const-1.c: New testcase.

Co-Authored-By: Jan Hubicka <hubicka@ucw.cz>

From-SVN: r158803

gcc/ChangeLog

@@ -1,3 +1,21 @@
+2010-04-27  Manuel López-Ibáñez  <manu@gcc.gnu.org>
+	    Jan Hubicka <hubicka@ucw.cz>
+
+	* doc/invoke.texi (-Wsuggest-attribute=const,
+	-Wsuggest-attribute=pure): Document.
+	* ipa-pure-const.c: Include toplev.h, intl.h and opts.h.
+	(function_always_visible_to_compiler_p,
+	suggest_attribute, warn_function_pure, warn_function_const):
+	New functions.
+	(check_call): Improve debug info.
+	(analyze_function): Do not check availability.
+	(add_new_function): Check availability.
+	(propagate): Output warnings.
+	(skip_function_for_local_pure_const): New function.
+	(local_pure_const): Use it; output warnings.
+	* common.opt (Wsuggest-attribute=const,
+	Wsuggest-attribute=pure): New.
+
 2010-04-27  Jakub Jelinek  <jakub@redhat.com>
 
 	* dwarf2out.c (def_cfa_1): After DW_CFA_def_cfa_expression

gcc/common.opt

@@ -180,6 +180,14 @@ Wstrict-overflow=
 Common Joined UInteger Var(warn_strict_overflow) Init(-1) Warning
 Warn about optimizations that assume that signed overflow is undefined
 
+Wsuggest-attribute=const
+Common Var(warn_suggest_attribute_const) Warning
+Warn about functions which might be candidates for __attribute__((const))
+
+Wsuggest-attribute=pure
+Common Var(warn_suggest_attribute_pure) Warning
+Warn about functions which might be candidates for __attribute__((pure))
+
 Wswitch
 Common Var(warn_switch) Warning
 Warn about enumerated switches, with no default, missing a case

gcc/doc/invoke.texi

@@ -257,6 +257,7 @@ Objective-C and Objective-C++ Dialects}.
 -Wsign-compare  -Wsign-conversion  -Wstack-protector @gol
 -Wstrict-aliasing -Wstrict-aliasing=n @gol
 -Wstrict-overflow -Wstrict-overflow=@var{n} @gol
+-Wsuggest-attribute=@r{[}const@r{|}pure@r{]} @gol
 -Wswitch  -Wswitch-default  -Wswitch-enum -Wsync-nand @gol
 -Wsystem-headers  -Wtrigraphs  -Wtype-limits  -Wundef  -Wuninitialized @gol
 -Wunknown-pragmas  -Wno-pragmas @gol
@@ -3621,11 +3622,36 @@ comparisons, so this warning level will give a very large number of
 false positives.
 @end table
 
+@item -Wsuggest-attribute=@r{[}const@r{|}pure@r{]}
+@opindex Wsuggest-attribute=
+@opindex Wno-suggest-attribute=
+Warn for cases where adding an attribute may be beneficial. The
+attributes currently supported are listed below.
+
+@table @gcctabopt
+@item -Wsuggest-attribute=pure
+@itemx -Wsuggest-attribute=const
+@opindex Wsuggest-attribute=pure
+@opindex Wno-suggest-attribute=pure
+@opindex Wsuggest-attribute=const
+@opindex Wno-suggest-attribute=const
+
+Warn about functions which might be candidates for attributes
+@code{pure} or @code{const}.  The compiler only warns for functions
+visible in other compilation units or if it cannot prove that the
+function returns normally. A function returns normally if it doesn't
+contain an infinite loop nor returns abnormally by throwing, calling
+@code{abort()} or trapping.  This analysis requires option
+@option{-fipa-pure-const}, which is enabled by default at @option{-O}
+and higher.  Higher optimization levels improve the accuracy of the
+analysis.
+@end table
+
 @item -Warray-bounds
 @opindex Wno-array-bounds
 @opindex Warray-bounds
 This option is only active when @option{-ftree-vrp} is active
-(default for -O2 and above). It warns about subscripts to arrays
+(default for @option{-O2} and above). It warns about subscripts to arrays
 that are always out of bounds. This warning is enabled by @option{-Wall}.
 
 @item -Wno-div-by-zero

gcc/ipa-pure-const.c

@@ -49,12 +49,15 @@ along with GCC; see the file COPYING3.  If not see
 #include "output.h"
 #include "flags.h"
 #include "timevar.h"
+#include "toplev.h"
 #include "diagnostic.h"
 #include "langhooks.h"
 #include "target.h"
 #include "lto-streamer.h"
 #include "cfgloop.h"
 #include "tree-scalar-evolution.h"
+#include "intl.h"
+#include "opts.h"
 
 static struct pointer_set_t *visited_nodes;
 
@@ -106,6 +109,71 @@ static struct cgraph_node_hook_list *function_insertion_hook_holder;
 static struct cgraph_2node_hook_list *node_duplication_hook_holder;
 static struct cgraph_node_hook_list *node_removal_hook_holder;
 
+/* Try to guess if function body will always be visible to compiler
+   when compiling the call and whether compiler will be able
+   to propagate the information by itself.  */
+
+static bool
+function_always_visible_to_compiler_p (tree decl)
+{
+  return (!TREE_PUBLIC (decl) || DECL_DECLARED_INLINE_P (decl));
+}
+
+/* Emit suggestion about attribute ATTRIB_NAME for DECL.  KNOWN_FINITE
+   is true if the function is known to be finite.  The diagnostic is
+   controlled by OPTION.  WARNED_ABOUT is a pointer_set unique for
+   OPTION, this function may initialize it and it is always returned
+   by the function.  */
+
+static struct pointer_set_t *
+suggest_attribute (int option, tree decl, bool known_finite,
+		   struct pointer_set_t *warned_about,
+		   const char * attrib_name)
+{
+  if (!option_enabled (option))
+    return warned_about;
+  if (TREE_THIS_VOLATILE (decl)
+      || (known_finite && function_always_visible_to_compiler_p (decl)))
+    return warned_about;
+
+  if (!warned_about)
+    warned_about = pointer_set_create (); 
+  if (pointer_set_contains (warned_about, decl))
+    return warned_about;
+  pointer_set_insert (warned_about, decl);
+  warning_at (DECL_SOURCE_LOCATION (decl),
+	      option,
+	      known_finite
+	      ? _("function might be candidate for attribute %<%s%>")
+	      : _("function might be candidate for attribute %<%s%>"
+		  " if it is known to return normally"), attrib_name);
+  return warned_about;
+}
+
+/* Emit suggestion about __attribute_((pure)) for DECL.  KNOWN_FINITE
+   is true if the function is known to be finite.  */
+
+static void
+warn_function_pure (tree decl, bool known_finite)
+{
+  static struct pointer_set_t *warned_about;
+
+  warned_about 
+    = suggest_attribute (OPT_Wsuggest_attribute_pure, decl,
+			 known_finite, warned_about, "pure");
+}
+
+/* Emit suggestion about __attribute_((const)) for DECL.  KNOWN_FINITE
+   is true if the function is known to be finite.  */
+
+static void
+warn_function_const (tree decl, bool known_finite)
+{
+  static struct pointer_set_t *warned_about;
+  warned_about 
+    = suggest_attribute (OPT_Wsuggest_attribute_const, decl,
+			 known_finite, warned_about, "const");
+}
 /* Init the function state.  */
 
 static void
@@ -325,7 +393,11 @@ check_call (funct_state local, gimple call, bool ipa)
 
   /* When not in IPA mode, we can still handle self recursion.  */
   if (!ipa && callee_t == current_function_decl)
-    local->looping = true;
+    {
+      if (dump_file)
+        fprintf (dump_file, "    Recursive call can loop.\n");
+      local->looping = true;
+    }
   /* Either calle is unknown or we are doing local analysis.
      Look to see if there are any bits available for the callee (such as by
      declaration or because it is builtin) and process solely on the basis of
@@ -353,12 +425,20 @@ check_call (funct_state local, gimple call, bool ipa)
       if (flags & ECF_CONST)
 	{
           if (callee_t && DECL_LOOPING_CONST_OR_PURE_P (callee_t))
-            local->looping = true;
+	    {
+	      if (dump_file)
+		fprintf (dump_file, "    calls looping pure.\n");
+              local->looping = true;
+	    }
 	 }
       else if (flags & ECF_PURE)
 	{
           if (callee_t && DECL_LOOPING_CONST_OR_PURE_P (callee_t))
-            local->looping = true;
+	    {
+	      if (dump_file)
+		fprintf (dump_file, "    calls looping const.\n");
+              local->looping = true;
+	    }
 	  if (dump_file)
 	    fprintf (dump_file, "    pure function call in not const\n");
 	  if (local->pure_const_state == IPA_CONST)
@@ -604,7 +684,8 @@ add_new_function (struct cgraph_node *node, void *data ATTRIBUTE_UNUSED)
      since all we would be interested in are the addressof
      operations.  */
   visited_nodes = pointer_set_create ();
-  set_function_state (node, analyze_function (node, true));
+  if (cgraph_function_body_availability (node) > AVAIL_OVERWRITABLE)
+    set_function_state (node, analyze_function (node, true));
   pointer_set_destroy (visited_nodes);
   visited_nodes = NULL;
 }
@@ -938,19 +1019,27 @@ propagate (void)
 	  switch (this_state)
 	    {
 	    case IPA_CONST:
-	      if (!TREE_READONLY (w->decl) && dump_file)
+	      if (!TREE_READONLY (w->decl))
-		fprintf (dump_file, "Function found to be %sconst: %s\n",
+		{
-			 this_looping ? "looping " : "",
+		  warn_function_const (w->decl, !this_looping);
-			 cgraph_node_name (w));
+		  if (dump_file)
+		    fprintf (dump_file, "Function found to be %sconst: %s\n",
+			     this_looping ? "looping " : "",
+			     cgraph_node_name (w));
+		}
 	      cgraph_set_readonly_flag (w, true);
 	      cgraph_set_looping_const_or_pure_flag (w, this_looping);
 	      break;
 
 	    case IPA_PURE:
-	      if (!DECL_PURE_P (w->decl) && dump_file)
+	      if (!DECL_PURE_P (w->decl))
-		fprintf (dump_file, "Function found to be %spure: %s\n",
+		{
-			 this_looping ? "looping " : "",
+		  warn_function_pure (w->decl, !this_looping);
-			 cgraph_node_name (w));
+		  if (dump_file)
+		    fprintf (dump_file, "Function found to be %spure: %s\n",
+			     this_looping ? "looping " : "",
+			     cgraph_node_name (w));
+		}
 	      cgraph_set_pure_flag (w, true);
 	      cgraph_set_looping_const_or_pure_flag (w, this_looping);
 	      break;
@@ -1103,6 +1192,29 @@ struct ipa_opt_pass_d pass_ipa_pure_const =
  NULL					/* variable_transform */
 };
 
+/* Return true if function should be skipped for local pure const analysis.  */
+
+static bool
+skip_function_for_local_pure_const (struct cgraph_node *node)
+{
+  /* Because we do not schedule pass_fixup_cfg over whole program after early optimizations
+     we must not promote functions that are called by already processed functions.  */
+
+  if (function_called_by_processed_nodes_p ())
+    {
+      if (dump_file)
+        fprintf (dump_file, "Function called in recursive cycle; ignoring\n");
+      return true;
+    }
+  if (cgraph_function_body_availability (node) <= AVAIL_OVERWRITABLE)
+    {
+      if (dump_file)
+        fprintf (dump_file, "Function is not available or overwrittable; not analyzing.\n");
+      return true;
+    }
+  return false;
+}
+
 /* Simple local pass for pure const discovery reusing the analysis from
    ipa_pure_const.   This pass is effective when executed together with
    other optimization passes in early optimization pass queue.  */
@@ -1112,25 +1224,15 @@ local_pure_const (void)
 {
   bool changed = false;
   funct_state l;
+  bool skip;
   struct cgraph_node *node;
 
-  /* Because we do not schedule pass_fixup_cfg over whole program after early optimizations
-     we must not promote functions that are called by already processed functions.  */
-
-  if (function_called_by_processed_nodes_p ())
-    {
-      if (dump_file)
-        fprintf (dump_file, "Function called in recursive cycle; ignoring\n");
-      return 0;
-    }
   node = cgraph_node (current_function_decl);
-  if (cgraph_function_body_availability (node) <= AVAIL_OVERWRITABLE)
+  skip = skip_function_for_local_pure_const (node);
-    {
+  if (!warn_suggest_attribute_const
-      if (dump_file)
+      && !warn_suggest_attribute_pure
-        fprintf (dump_file, "Function has wrong visibility; ignoring\n");
+      && skip)
-      return 0;
+    return 0;
-    }
-
   l = analyze_function (node, false);
 
   switch (l->pure_const_state)
@@ -1138,9 +1240,13 @@ local_pure_const (void)
     case IPA_CONST:
       if (!TREE_READONLY (current_function_decl))
 	{
-	  cgraph_set_readonly_flag (node, true);
+	  warn_function_const (current_function_decl, !l->looping);
-	  cgraph_set_looping_const_or_pure_flag (node, l->looping);
+	  if (!skip)
-	  changed = true;
+	    {
+	      cgraph_set_readonly_flag (node, true);
+	      cgraph_set_looping_const_or_pure_flag (node, l->looping);
+	      changed = true;
+	    }
 	  if (dump_file)
 	    fprintf (dump_file, "Function found to be %sconst: %s\n",
 		     l->looping ? "looping " : "",
@@ -1150,8 +1256,11 @@ local_pure_const (void)
       else if (DECL_LOOPING_CONST_OR_PURE_P (current_function_decl)
 	       && !l->looping)
 	{
-	  cgraph_set_looping_const_or_pure_flag (node, false);
+	  if (!skip)
-	  changed = true;
+	    {
+	      cgraph_set_looping_const_or_pure_flag (node, false);
+	      changed = true;
+	    }
 	  if (dump_file)
 	    fprintf (dump_file, "Function found to be non-looping: %s\n",
 		     lang_hooks.decl_printable_name (current_function_decl,
@@ -1160,11 +1269,15 @@ local_pure_const (void)
       break;
 
     case IPA_PURE:
-      if (!TREE_READONLY (current_function_decl))
+      if (!DECL_PURE_P (current_function_decl))
 	{
-	  cgraph_set_pure_flag (node, true);
+	  if (!skip)
-	  cgraph_set_looping_const_or_pure_flag (node, l->looping);
+	    {
-	  changed = true;
+	      cgraph_set_pure_flag (node, true);
+	      cgraph_set_looping_const_or_pure_flag (node, l->looping);
+	      changed = true;
+	    }
+	  warn_function_pure (current_function_decl, !l->looping);
 	  if (dump_file)
 	    fprintf (dump_file, "Function found to be %spure: %s\n",
 		     l->looping ? "looping " : "",
@@ -1174,8 +1287,11 @@ local_pure_const (void)
       else if (DECL_LOOPING_CONST_OR_PURE_P (current_function_decl)
 	       && !l->looping)
 	{
-	  cgraph_set_looping_const_or_pure_flag (node, false);
+	  if (!skip)
-	  changed = true;
+	    {
+	      cgraph_set_looping_const_or_pure_flag (node, false);
+	      changed = true;
+	    }
 	  if (dump_file)
 	    fprintf (dump_file, "Function found to be non-looping: %s\n",
 		     lang_hooks.decl_printable_name (current_function_decl,

gcc/testsuite/ChangeLog

@@ -1,3 +1,9 @@
+2010-04-27  Manuel López-Ibáñez  <manu@gcc.gnu.org>
+	    Jan Hubicka <hubicka@ucw.cz>
+
+	* gcc.dg/pure-2.c: New testcase.
+	* gcc.dg/const-1.c: New testcase.
+
 2010-04-27  Jason Merrill  <jason@redhat.com>
 
 	* g++.dg/cpp0x/lambda/lambda-ice1.C: New.

gcc/testsuite/gcc.dg/const-1.c

@@ -0,0 +1,57 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -Wsuggest-attribute=const" } */
+
+extern int extern_const(int a) __attribute__ ((const));
+
+/* Trivial.  */
+int
+foo1(int a)  /* { dg-bogus "normally" "detect const candidate" } */
+{ /* { dg-warning "const" "detect const candidate" { target *-*-* } "8" } */ 
+  return extern_const (a);
+}
+
+/* Loops known to be normally and extern const calls should be safe.  */
+
+int __attribute__ ((noinline))
+foo2(int n)  /* { dg-bogus "normally" "detect const candidate" } */
+{ /* { dg-warning "const" "detect const candidate" { target *-*-* } "16" } */
+  int ret = 0;
+  int i;
+  for (i=0; i<n; i++)
+    ret+=extern_const (i);
+  return ret;
+}
+
+/* No warning here; we can work it by ourselves.  */
+static int __attribute__ ((noinline))
+foo2b(int n)
+{
+  int ret = 0;
+  int i;
+  for (i=0; i<n; i++)
+    ret+=extern_const (i);
+  return ret;
+}
+
+/* Unbounded loops are not safe.  */
+static int __attribute__ ((noinline))
+foo3(int n)  /* { dg-warning "const\[^\n\]* normally" "detect const candidate" } */
+{
+  int ret = 0;
+  int i;
+  for (i=0; extern_const (i+n); n++)
+    ret+=extern_const (i);
+  return ret;
+}
+
+int
+foo4(int n) /* { dg-warning "const\[^\n\]* normally" "detect const candidate" } */
+{
+  return foo3(n) + foo2b(n);
+} 
+
+int
+foo5(int n)  /* { dg-bogus "normally" "detect const candidate" } */
+{ /* { dg-warning "const" "detect const candidate" { target *-*-* } "54" } */
+  return foo2(n);
+} 

gcc/testsuite/gcc.dg/pure-2.c

@@ -0,0 +1,57 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -Wsuggest-attribute=pure" } */
+
+extern int extern_const(int a) __attribute__ ((pure));
+extern int v;
+
+/* Trivial.  */
+int
+foo1(int a)  /* { dg-bogus "normally" "detect pure candidate" } */
+{ /* { dg-warning "pure" "detect pure candidate" { target *-*-* } "9" } */
+  return v;
+}
+
+/* Loops known to be normally and extern const calls should be safe.  */
+int __attribute__ ((noinline))
+foo2(int n)  /* { dg-bogus "normally" "detect pure candidate" } */
+{ /* { dg-warning "pure" "detect pure candidate" { target *-*-* } "16" } */
+  int ret = 0;
+  int i;
+  for (i=0; i<n; i++)
+    ret+=extern_const (i);
+  return ret;
+}
+
+/* No warning here; we can work it by ourselves.  */
+static int __attribute__ ((noinline))
+foo2b(int n)
+{
+  int ret = 0;
+  int i;
+  for (i=0; i<n; i++)
+    ret+=extern_const (i);
+  return ret;
+}
+
+/* Unbounded loops are not safe.  */
+static int __attribute__ ((noinline))
+foo3(int n) /* { dg-warning "pure\[^\n\]* normally" "detect pure candidate" } */
+{
+  int ret = 0;
+  int i;
+  for (i=0; extern_const (i+n); n++)
+    ret+=extern_const (i);
+  return ret;
+}
+
+int
+foo4(int n)  /* { dg-warning "pure\[^\n\]* normally" "detect pure candidate" } */
+{
+  return foo3(n) + foo2b(n);
+}
+
+int
+foo5(int n)  /* { dg-bogus "normally" "detect pure candidate" } */
+{  /* { dg-warning "pure" "detect pure candidate" { target *-*-* } "54" } */
+  return foo2(n);
+}