[tree-ssa] Fix handling of call clobbered pointer references [patch]

[Diego Novillo <dnovillo at redhat dot com>]

This patch fixes a bug exposed by the coalescer in the
SSA->normal pass.  When passing a pointer 'p'  to a clobbering
function, the compiler was emitting a VDEF operand for '*p'.
This is not strictly wrong, but it's unnecessary because we
already model call-clobbering semantics using .global_var, so if
'*p' is call-clobbered, uses of '*p' after the call are already
reached by the definition of .global_var at the call site.

This was also causing the coalescer to think that there were two
versions of '*p' with overlapping live ranges:

	p = 0;			p_3 = 0;
	...			...
	foo (p);		# (*p)_5 = VDEF <(*p)_4>
				# .GLOBAL_VAR_7 = VDEF <.GLOBAL_VAR_6>
				foo (p_3);

After constant propagation, the above code is converted to:

	(void)0;
	...
	# .GLOBAL_VAR_7 = VDEF <.GLOBAL_VAR_6>
	foo (0);

Suddenly, the VDEF for (*p)_5 disappears because the call to
foo() is not referencing 'p' anymore.  Successive uses of (*p)_5
were not being updated and the coalescer was tricked into
thinking that (*p)_5 was live all the way to the beginning of the
function.  Since we are currently not allowing overlapping live
ranges, the check at tree-ssa.c:assign_vars was causing an abort.

Another side effect of the patch is that we emit less voperands
for calls to clobbering functions, which has a positive effect on
compile time.

I also found out that we are not marking __builtin_constant_p as
a constant function.  This was also confusing the SSA->normal
pass because calls to __builtin_constant_p were being constant
folded, leaving dangling VDEFs for .global_var.  I'll be
submitting this fix for mainline shortly.

Bootstrapped and tested x86, x86-64 and ppc.


Diego.


	* builtins.def (BUILTIN_CONSTANT_P): Mark as constant.

	* tree-dfa.c (get_expr_operands): Do not add VDEF operands for
	dereferenced pointers at call sites.
	* tree-ssa.c (assign_vars): Abort if we couldn't coalesce all the
	versions together.

Index: builtins.def
===================================================================
RCS file: /cvs/gcc/gcc/gcc/builtins.def,v
retrieving revision 1.29.2.13
diff -d -u -p -r1.29.2.13 builtins.def
--- builtins.def	9 Apr 2003 19:27:17 -0000	1.29.2.13
+++ builtins.def	29 Apr 2003 18:39:27 -0000
@@ -670,7 +670,7 @@ DEF_GCC_BUILTIN(BUILT_IN_ARGS_INFO,
 DEF_GCC_BUILTIN(BUILT_IN_CONSTANT_P, 
 		"__builtin_constant_p", 
 		BT_FN_INT_VAR,
-		ATTR_NULL)
+		ATTR_CONST_NOTHROW_LIST)
 DEF_GCC_BUILTIN(BUILT_IN_FRAME_ADDRESS,
 		"__builtin_frame_address",
 		BT_FN_PTR_UNSIGNED,
Index: tree-dfa.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/Attic/tree-dfa.c,v
retrieving revision 1.1.4.100
diff -d -u -p -r1.1.4.100 tree-dfa.c
--- tree-dfa.c	25 Apr 2003 23:42:38 -0000	1.1.4.100
+++ tree-dfa.c	29 Apr 2003 18:39:28 -0000
@@ -420,7 +420,6 @@ get_expr_operands (stmt, expr_p, flags, 
      (See find_vars_r).  */
   if (code == CALL_EXPR)
     {
-      int flags;
       tree op;
       bool may_clobber = call_may_clobber (expr);
 
@@ -428,11 +427,9 @@ get_expr_operands (stmt, expr_p, flags, 
       get_expr_operands (stmt, &TREE_OPERAND (expr, 0), opf_none, prev_vops);
 
       /* Add all the arguments to the function.  For every pointer argument
-	 to the function, add a VUSE for its dereferenced pointer (if the
-	 function is pure or const) or a VDEF for its dereferenced pointer
-	 (if the function may clobber).  This is to address the following
-	 problem: Suppose that function 'foo' receives a pointer to a local
-	 variable:
+	 to the function, add a VUSE for its dereferenced pointer.  This is
+	 to address the following problem: Suppose that function 'foo'
+	 receives a pointer to a local variable:
 
 	    int foo (int *x)
 	    {
@@ -451,26 +448,28 @@ get_expr_operands (stmt, expr_p, flags, 
 	 to kill the assignment to 'i' because it's never used in bar().
 	 To address this problem, we add a VUSE<*p> at the call site of
 	 foo().  */
-      flags = opf_force_vop | opf_ignore_bp;
       for (op = TREE_OPERAND (expr, 1); op; op = TREE_CHAIN (op))
 	{
 	  tree arg = TREE_VALUE (op);
 
 	  add_stmt_operand (&TREE_VALUE (op), stmt, opf_none, prev_vops);
 
-	  /* Add a VUSE<*p> or VDEF<*p> for every pointer p passed in the
-	     argument list (see note above).  */
-	  if (SSA_DECL_P (arg)
+	  /* If the function may not clobber locals, add a VUSE<*p> for
+	     every pointer p passed in the argument list (see note above).
+	     It's not necessary to do this for clobbering calls because we
+	     will be adding a VDEF for .GLOBAL_VAR for this call.  */
+	  if (!may_clobber
+	      && SSA_DECL_P (arg)
 	      && POINTER_TYPE_P (TREE_TYPE (arg)))
 	    {
 	      tree deref = indirect_ref (arg);
-	      int clobber_arg = (may_clobber && !TREE_READONLY (deref));
 
 	      /* By default, adding a reference to an INDIRECT_REF
 		 variable, adds a VUSE of the base pointer.  Since we have
 		 already added a real USE for the pointer, we don't need to
 		 add a VUSE for it as well.  */
-	      add_stmt_operand (&deref, stmt, flags | clobber_arg, prev_vops);
+	      add_stmt_operand (&deref, stmt, opf_force_vop|opf_ignore_bp,
+				prev_vops);
 	    }
 	}
 
Index: tree-ssa.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/Attic/tree-ssa.c,v
retrieving revision 1.1.4.69
diff -d -u -p -r1.1.4.69 tree-ssa.c
--- tree-ssa.c	26 Apr 2003 03:04:04 -0000	1.1.4.69
+++ tree-ssa.c	29 Apr 2003 18:39:28 -0000
@@ -1397,19 +1397,16 @@ assign_vars (map)
 	      change_partition_var (map, var, i);
 	      continue;
 	    }
-/*
-  TODO: enable this line of code once the bug which removed VDEFS in 
-  get_stmt_operands() is fixed.  
-
-  This abort() should then verify that we always coalesce all versions of 
-  a variable together. The abort() is then removed when we allow overlapping
-  live ranges.  
 
-abort();
+	  /* Since we still don't have passes that create overlapping live
+	     ranges, the code above should've coalesced all the versions of
+	     the variable together.  */
+	  abort();
+#if 0
 	  var = create_temp (t);
-*/
 	  change_partition_var (map, var, i);
 	  ann = var_ann (var);
+#endif
 	}
     }