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]

[RFC]: patch to detect invalid and missing ATTRIBUTE const/pure


Ok so I modified gcc as follows to detect invalid attempts to use
attribute const/pure, and I also added flags to have gcc suggest
places to add the attributes.  It survived bootstrap on
sparc-solaris2.7 with -Wmissing-pure -Wmissing-const added to
BOOT_CFLAGS and yielded several hundred suggested functions where
attributes should be added.

A cursory examination seems to indicate that its working correctly but
I'll have to look in each function individually to make sure.  (One
nice side effect is that its very easy to write positive and negative
testsuite cases for const/pure detection now that it outputs these
diagnostics.)

However I noticed something which may present a slight issue.  When I
bootstrap with checking on I get 412 suggestions, when I disable
checking, I get 496.

Why the discrepancy?  My guess is that with tree checking enabled,
lots of the functions using `tree' macros that would have been "pure"
now have a path which calls `tree_check_failed' which is a noreturn
function.  This "side effect" means that it is no longer pure.  While
gcc is pedantically correct in making the distinction, I think perhaps
it isn't "what we want" in practice.

Another case (regardless of checking) is host_integerp vs
tree_low_cst.  The former is considered "pure" whereas the latter
isn't I think because it calls abort when passed invalid arguments.
Nevertheless, I would consider tree_low_cst as much a candidate for
CSE as host_integerp if passed the same valid arguments which in
practice gcc should always do.  If not, well then it'll abort on the
first call and I don't care that the second was optimized away.

The patch below is merely for reference and comment at this point.
Obviously it'll need a ChangeLog, documentation and testcases.  But
for now let me know your thoughts on the patch and the issue I raised
above.

		Thanks,
		--Kaveh


diff -rcp orig/egcc-CVS20020321/gcc/alias.c egcc-CVS20020321/gcc/alias.c
*** orig/egcc-CVS20020321/gcc/alias.c	Thu Mar 21 07:00:26 2002
--- egcc-CVS20020321/gcc/alias.c	Thu Mar 21 14:33:07 2002
*************** mark_constant_function ()
*** 2551,2568 ****
    rtx insn;
    int nonlocal_memory_referenced;
  
!   if (TREE_PUBLIC (current_function_decl)
!       || TREE_READONLY (current_function_decl)
!       || DECL_IS_PURE (current_function_decl)
!       || TREE_THIS_VOLATILE (current_function_decl)
        || TYPE_MODE (TREE_TYPE (current_function_decl)) == VOIDmode
!       || current_function_has_nonlocal_goto)
!     return;
! 
!   /* A loop might not return which counts as a side effect.  */
!   if (mark_dfs_back_edges ())
!     return;
  
    nonlocal_memory_referenced = 0;
  
    init_alias_analysis ();
--- 2551,2570 ----
    rtx insn;
    int nonlocal_memory_referenced;
  
!   if (TREE_THIS_VOLATILE (current_function_decl)
        || TYPE_MODE (TREE_TYPE (current_function_decl)) == VOIDmode
!       || current_function_has_nonlocal_goto
!       /* A loop might not return which counts as a side effect.  */
!       || mark_dfs_back_edges ())
!     {
!       if (TREE_READONLY (current_function_decl))
! 	warning ("function invalid for attribute `const'");
!       else if (DECL_IS_PURE (current_function_decl))
! 	warning ("function invalid for attribute `pure'");
  
+       return;
+     }
+   
    nonlocal_memory_referenced = 0;
  
    init_alias_analysis ();
*************** mark_constant_function ()
*** 2587,2597 ****
    /* Mark the function.  */
    
    if (insn)
!     ;
    else if (nonlocal_memory_referenced)
!     DECL_IS_PURE (current_function_decl) = 1;
    else
!     TREE_READONLY (current_function_decl) = 1;
  }
  
  
--- 2589,2620 ----
    /* Mark the function.  */
    
    if (insn)
!     {
!       if (TREE_READONLY (current_function_decl))
! 	warning ("function invalid for attribute `const'");
!       else if (DECL_IS_PURE (current_function_decl))
! 	warning ("function invalid for attribute `pure'");
!     }
    else if (nonlocal_memory_referenced)
!     {
!       if (TREE_READONLY (current_function_decl))
! 	warning ("function invalid for attribute `const'");
!       if (! DECL_IS_PURE (current_function_decl))
!         {
! 	  if (warn_missing_pure)
! 	    warning ("function may be candidate for attribute `pure'");
! 	  DECL_IS_PURE (current_function_decl) = 1;
! 	}
!     }
    else
!     {
!       if (! TREE_READONLY (current_function_decl))
!         {
! 	  if (warn_missing_const)
! 	    warning ("function may be candidate for attribute `const'");
! 	  TREE_READONLY (current_function_decl) = 1;
! 	}
!     }
  }
  
  
diff -rcp orig/egcc-CVS20020321/gcc/flags.h egcc-CVS20020321/gcc/flags.h
*** orig/egcc-CVS20020321/gcc/flags.h	Thu Mar  7 16:30:20 2002
--- egcc-CVS20020321/gcc/flags.h	Thu Mar 21 14:27:57 2002
*************** extern int warn_return_type;
*** 139,144 ****
--- 139,152 ----
  
  extern int warn_missing_noreturn;
  
+ /* Warn about functions which might be candidates for attribute const.  */
+ 
+ extern int warn_missing_const;
+ 
+ /* Warn about functions which might be candidates for attribute pure.  */
+ 
+ extern int warn_missing_pure;
+ 
  /* Nonzero means warn about pointer casts that increase the required
     alignment of the target type (and might therefore lead to a crash
     due to a misaligned access).  */
diff -rcp orig/egcc-CVS20020321/gcc/toplev.c egcc-CVS20020321/gcc/toplev.c
*** orig/egcc-CVS20020321/gcc/toplev.c	Wed Mar 20 07:00:34 2002
--- egcc-CVS20020321/gcc/toplev.c	Thu Mar 21 14:52:17 2002
*************** int warn_disabled_optimization;
*** 1425,1430 ****
--- 1425,1438 ----
  
  int warn_missing_noreturn;
  
+ /* Warn about functions which might be candidates for attribute const.  */
+ 
+ int warn_missing_const;
+ 
+ /* Warn about functions which might be candidates for attribute pure.  */
+ 
+ int warn_missing_pure;
+ 
  /* Nonzero means warn about uses of __attribute__((deprecated)) 
     declarations.  */
  
*************** static const lang_independent_options W_
*** 1471,1477 ****
    {"deprecated-declarations", &warn_deprecated_decl, 1,
     N_("Warn about uses of __attribute__((deprecated)) declarations") },
    {"missing-noreturn", &warn_missing_noreturn, 1,
!    N_("Warn about functions which might be candidates for attribute noreturn") }
  };
  
  void
--- 1479,1489 ----
    {"deprecated-declarations", &warn_deprecated_decl, 1,
     N_("Warn about uses of __attribute__((deprecated)) declarations") },
    {"missing-noreturn", &warn_missing_noreturn, 1,
!    N_("Warn about functions which might be candidates for attribute noreturn") },
!   {"missing-const", &warn_missing_const, 1,
!    N_("Warn about functions which might be candidates for attribute const") },
!   {"missing-pure", &warn_missing_pure, 1,
!    N_("Warn about functions which might be candidates for attribute pure") }
  };
  
  void


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