Re: [tree-ssa] rfa [long]: mudflap prototype

[Diego Novillo <dnovillo at redhat dot com>]

On Thu, 08 Aug 2002, Frank Ch. Eigler wrote:

> Attached is the prototype mudflap instrumentation (as gcc patches)
> and runtime (as new files for .../libmudflap).  To use it, build
>
I think it would be easier for this to be checked-in the branch
and let people try it out.  You had bootstrapped with mudflap
disabled, right?

Could you add some documentation on the approach?  Either in the
code or in the tree-ssa page.  Also, an entry in doc/invoke.texi
should be added.

It seems like mudflap is doing some duplicated work.  Some of the
information you gather about variables is already gathered by
tree-dfa.c and tree-ssa.c.  It'd be better if you used that
instead of re-doing their work.


> --- gcc/c-pretty-print.c	19 Jul 2002 17:29:24 -0000	1.1.4.2
> +++ gcc/c-pretty-print.c	8 Aug 2002 22:46:08 -0000
> @@ -999,6 +999,15 @@
>        print_declaration (buffer, TREE_OPERAND (node, 0), spc);
>        break;
>  
> +    case CLEANUP_STMT:
> +      output_add_string (buffer, "__cleanup (");
>
Better render it as 'CLEANUP <...>'.  Otherwise it looks too much
like a function call (yes, I'm changing how we render other nodes
as well, this is a new change).

> --- gcc/c-simplify.c	7 Aug 2002 16:02:57 -0000	1.1.4.9
> +++ gcc/c-simplify.c	8 Aug 2002 22:46:09 -0000
> @@ -255,6 +255,11 @@
>  	  simplify_decl_stmt (stmt, &pre, &post, &post);
>  	  break;
>  
> +	case CLEANUP_STMT:
> +	  /* XXX: need to clean up CLEANUP_STMT.  Idea: turn it into
> +	     an statement-expression and simplify that.  */
> +	  break;
> +
>
Yes.  Would simplify_expr_stmt (TREE_OPERAND (stmt, 1)) work?
Same thing with the first operand (calling simplify_decl).

> RCS file: /cvs/gcc/gcc/gcc/Attic/tree-dfa.c,v
> retrieving revision 1.1.4.3
> diff -u -r1.1.4.3 tree-dfa.c
> --- gcc/tree-dfa.c	7 Aug 2002 16:02:58 -0000	1.1.4.3
> +++ gcc/tree-dfa.c	8 Aug 2002 22:46:10 -0000
> @@ -1,5 +1,5 @@
>  /* Data flow functions for trees.
> -   Copyright (C) 2001 Free Software Foundation, Inc.
> +   Copyright (C) 2001, 2002 Free Software Foundation, Inc.
>     Contributed by Diego Novillo <dnovillo@redhat.com>
>  
>  This file is part of GNU CC.
> @@ -181,6 +181,16 @@
>  	  if (DECL_INITIAL (decl))
>  	    find_refs_in_expr (decl, VARDEF, bb, t, t);
>  	}
> +      break;
> +
> +    case CLEANUP_STMT:
> +      if (TREE_CODE (CLEANUP_DECL (t)) == VAR_DECL)
> +	{
> +	  tree decl = TREE_OPERAND (t, 0);
> +	  if (DECL_INITIAL (decl))
> +	    find_refs_in_expr (decl, VARDEF, bb, t, t);
> +	}
> +      find_refs_in_expr (CLEANUP_EXPR (t), VARUSE, bb, t, t);
>        break;
>  
Hmm, tree-dfa.c is about to get very dumb.  It will only deal
with SIMPLE-ified trees.  But for now, we can get this in.

> Index: gcc/tree-mudflap.c
> ===================================================================
> RCS file: gcc/tree-mudflap.c
> diff -N gcc/tree-mudflap.c
> --- /dev/null	1 Jan 1970 00:00:00 -0000
> +++ gcc/tree-mudflap.c	8 Aug 2002 22:46:10 -0000
> @@ -0,0 +1,819 @@
> [ ... ]
> +/* {{{ Internal function decls */
> +
>
The problem with folding markers is that not everyone uses them,
so patches tend to disrupt them.  I decided it wasn't worth the
aggravation.  But if you want to keep them in, go ahead.

> +  if (getenv ("UNPARSE"))  /* XXX */
>
Eeek.  Suggest adding a -fdump-tree-... flag.  Check how
-fdump-tree-{ssa,original,simple} work.

> +/* Initialize the global tree nodes that correspond to mf-runtime.h
> +   declarations.  */
> +static void
> +mf_init_extern_trees ()
> +{
>
Maybe a few comments sprinkled in here to tell what's going on?

> +/* As a tree traversal function, maintain a scope stack for the
> +   current traversal stage, so that each DECL_STMT knows its enclosing
> +   COMPOUND_STMT.  For appropriate DECL_STMTs, perform the mudflap
> +   lifetime-tracking transform.
>
You may not need this.  This is known after building the CFG.
Each decl_stmt knows which compound_stmt encloses its basic
block.

> +/* As a tree traversal function, find the first expression node within
> +   the given tree that is an ADDR_EXPR or ARRAY_REF of the given
> +   VAR_DECL.
> +*/
> +static tree
> +mx_xfn_find_addrof (t, continue_p, data)
> +     tree *t;
> +     int *continue_p;
> +     void *data;
> +{
>
Also known after tree-dfa.c.  You have a list of all variables
referenced in the code, together with the type of reference,
which expression and which statement references them.

> +/* Find and return any instance of an ADDR_EXPR tree referring to decl
> +   under the given statement.  */
> +static tree
> +mf_find_addrof (stmt, decl)
> +     tree stmt;
> +     tree decl;
> +{
> +  return walk_tree_without_duplicates (& stmt, mx_xfn_find_addrof, decl);
> +}
> +
Ditto.

> Index: gcc/varasm.c
> ===================================================================
> RCS file: /cvs/gcc/gcc/gcc/varasm.c,v
> retrieving revision 1.295.2.2
> diff -u -r1.295.2.2 varasm.c
> --- gcc/varasm.c	5 Aug 2002 18:14:21 -0000	1.295.2.2
> +++ gcc/varasm.c	8 Aug 2002 22:46:11 -0000
> @@ -47,6 +47,8 @@
>  #include "tm_p.h"
>  #include "debug.h"
>  #include "target.h"
> +#include "tree-mudflap.h"
> +
>  
>  #ifdef XCOFF_DEBUGGING_INFO
>  #include "xcoffout.h"		/* Needed for external data
> @@ -960,6 +962,10 @@
>       If the name is changed, the macro ASM_OUTPUT_LABELREF
>       will have to know how to strip this information.  */
>    (* targetm.encode_section_info) (decl, true);
> +
> +  /* Make this function static known to the mudflap runtime.  */
> +  if (flag_mudflap && TREE_CODE (decl) == VAR_DECL)
> +    mudflap_enqueue_decl (decl, name);
>  }
>  
>  /* Make the rtl for variable VAR be volatile.
> @@ -2877,6 +2883,16 @@
>  		    : int_size_in_bytes (TREE_TYPE (exp))),
>  		   align);
>  
> +  if (flag_mudflap)
> +    {
> +      char label[200];
> +
> +      /* It's a waste to regenerate this string here.  The callers
> +	 almost always have a copy of the same string someplace.  */
> +      ASM_GENERATE_INTERNAL_LABEL (label, "LC", labelno);
> +
> +      mudflap_enqueue_constant (exp, ggc_strdup (label));
> +    }
>  }
>
You explained this to me once, but I forgot, sorry.  Why did you
need this?


Diego.