Fix handling of ASM_EXPR operands

Fix handling of ASM_EXPR operands

[Diego Novillo]

This is another fallout from the optimizer reorg I'm doing.  When an
ASM_EXPR clobbers memory it not only clobbers call-clobbered variables,
but it also clobbers addressable locals.  Furthermore, we don't know
what most of those are until after aliasing.

This produced SSA verification problems together with the other changes
I'm working on, but I couldn't replicate them in a clean tree.

Bootstrapped and tested x86, x86-64 and ppc.


Diego.


2004-07-08  Diego Novillo  <dnovillo@redhat.com>

	* tree-flow.h (addressable_vars): Declare.
	* tree-ssa-alias.c (addressable_vars): Define.
	(setup_pointers_and_addressables): Add addressable variables
	to addressable_vars.
	* tree-ssa-operands.c (get_stmt_operands): Move
	handling of ASM_EXPRs ...
	(get_asm_expr_operands): ... here.
	When the ASM_EXPR clobbers memory, also clobber addressable
	variables.
	* tree-ssa.c (init_tree_ssa): Initialize addressable_vars.
	(delete_tree_ssa): Reset addressable_vars.

Index: tree-flow.h
===================================================================
RCS file: /cvs/gcc/gcc/gcc/tree-flow.h,v
retrieving revision 2.19
diff -d -c -p -d -u -p -r2.19 tree-flow.h
--- tree-flow.h	7 Jul 2004 20:16:00 -0000	2.19
+++ tree-flow.h	8 Jul 2004 04:50:13 -0000
@@ -395,6 +395,10 @@ extern GTY(()) tree global_var;
    REFERENCED_VARS (I) is call-clobbered.  */
 extern bitmap call_clobbered_vars;
 
+/* Addressable variables in the function.  If bit I is set, then
+   REFERENCED_VARS (I) has had its address taken.  */
+extern bitmap addressable_vars;
+
 /* 'true' after aliases have been computed (see compute_may_aliases).  */
 extern bool aliases_computed_p;
 
Index: tree-ssa-alias.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/tree-ssa-alias.c,v
retrieving revision 2.8
diff -d -c -p -d -u -p -r2.8 tree-ssa-alias.c
--- tree-ssa-alias.c	16 Jun 2004 23:03:31 -0000	2.8
+++ tree-ssa-alias.c	8 Jul 2004 04:50:13 -0000
@@ -163,6 +163,10 @@ static struct ptr_info_def *get_ptr_info
    REFERENCED_VARS (I) is call-clobbered.  */
 bitmap call_clobbered_vars;
 
+/* Addressable variables in the function.  If bit I is set, then
+   REFERENCED_VARS (I) has had its address taken.  */
+bitmap addressable_vars;
+
 /* 'true' after aliases have been computed (see compute_may_aliases).  This
    is used by get_stmt_operands and its helpers to determine what to do
    when scanning an operand for a variable that may be aliased.  If
@@ -1196,6 +1200,14 @@ setup_pointers_and_addressables (struct 
 		 to rename VAR into SSA afterwards.  */
 	      bitmap_set_bit (vars_to_rename, v_ann->uid);
 	    }
+	  else
+	    {
+	      /* Add the variable to the set of addressables.  Mostly
+		 used when scanning operands for ASM_EXPRs that
+		 clobber memory.  In those cases, we need to clobber
+		 all call-clobbered variables and all addressables.  */
+	      bitmap_set_bit (addressable_vars, v_ann->uid);
+	    }
 	}
 
       /* Global variables and addressable locals may be aliased.  Create an
Index: tree-ssa-operands.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/tree-ssa-operands.c,v
retrieving revision 2.13
diff -d -c -p -d -u -p -r2.13 tree-ssa-operands.c
--- tree-ssa-operands.c	30 Jun 2004 21:11:28 -0000	2.13
+++ tree-ssa-operands.c	8 Jul 2004 04:50:13 -0000
@@ -80,6 +80,7 @@ typedef struct voperands_d 
 
 static void note_addressable (tree, stmt_ann_t);
 static void get_expr_operands (tree, tree *, int, voperands_t);
+static void get_asm_expr_operands (tree, voperands_t);
 static inline void append_def (tree *, tree);
 static inline void append_use (tree *, tree);
 static void append_v_may_def (tree, tree, voperands_t);
@@ -777,59 +778,7 @@ get_stmt_operands (tree stmt)
       break;
 
     case ASM_EXPR:
-      {
-	int noutputs = list_length (ASM_OUTPUTS (stmt));
-	const char **oconstraints
-	  = (const char **) alloca ((noutputs) * sizeof (const char *));
-	int i;
-	tree link;
-	const char *constraint;
-	bool allows_mem, allows_reg, is_inout;
-
-	for (i=0, link = ASM_OUTPUTS (stmt); link;
-	     ++i, link = TREE_CHAIN (link))
-	  {
-	    oconstraints[i] = constraint
-	      = TREE_STRING_POINTER (TREE_VALUE (TREE_PURPOSE (link)));
-	    parse_output_constraint (&constraint, i, 0, 0,
-				     &allows_mem, &allows_reg, &is_inout);
-	    if (allows_reg && is_inout)
-	      /* This should have been split in gimplify_asm_expr.  */
-	      abort ();
-
-	    if (!allows_reg && allows_mem)
-	      {
-		tree t = get_base_address (TREE_VALUE (link));
-		if (t && DECL_P (t))
-		  mark_call_clobbered (t);
-	      }
-
-	    get_expr_operands (stmt, &TREE_VALUE (link), opf_is_def,
-			       &prev_vops);
-	  }
-
-	for (link = ASM_INPUTS (stmt); link; link = TREE_CHAIN (link))
-	  {
-	    constraint
-	      = TREE_STRING_POINTER (TREE_VALUE (TREE_PURPOSE (link)));
-	    parse_input_constraint (&constraint, 0, 0, noutputs, 0,
-				    oconstraints, &allows_mem, &allows_reg);
-
-	    if (!allows_reg && allows_mem)
-	      {
-		tree t = get_base_address (TREE_VALUE (link));
-		if (t && DECL_P (t))
-		  mark_call_clobbered (t);
-	      }
-
-	    get_expr_operands (stmt, &TREE_VALUE (link), 0, &prev_vops);
-	  }
-
-	/* Clobber memory for asm ("" : : : "memory");  */
-	for (link = ASM_CLOBBERS (stmt); link; link = TREE_CHAIN (link))
-	  if (!strcmp (TREE_STRING_POINTER (TREE_VALUE (link)), "memory"))
-	    add_call_clobber_ops (stmt, &prev_vops);
-      }
+      get_asm_expr_operands (stmt, &prev_vops);
       break;
 
     case RETURN_EXPR:
@@ -1215,6 +1164,108 @@ get_expr_operands (tree stmt, tree *expr
   abort ();
 }
 
+/* Scan operands in ASM_EXPR STMT.  PREV_VOPS is as in
+   append_v_may_def and append_vuse.  */
+
+static void
+get_asm_expr_operands (tree stmt, voperands_t prev_vops)
+{
+  int noutputs = list_length (ASM_OUTPUTS (stmt));
+  const char **oconstraints
+    = (const char **) alloca ((noutputs) * sizeof (const char *));
+  int i;
+  tree link;
+  const char *constraint;
+  bool allows_mem, allows_reg, is_inout;
+  stmt_ann_t s_ann = stmt_ann (stmt);
+
+  for (i=0, link = ASM_OUTPUTS (stmt); link; ++i, link = TREE_CHAIN (link))
+    {
+      oconstraints[i] = constraint
+	= TREE_STRING_POINTER (TREE_VALUE (TREE_PURPOSE (link)));
+      parse_output_constraint (&constraint, i, 0, 0,
+	  &allows_mem, &allows_reg, &is_inout);
+
+#if defined ENABLE_CHECKING
+      /* This should have been split in gimplify_asm_expr.  */
+      if (allows_reg && is_inout)
+	abort ();
+#endif
+
+      /* Memory operands are addressable.  Note that STMT needs the
+	 address of this operand.  */
+      if (!allows_reg && allows_mem)
+	{
+	  tree t = get_base_address (TREE_VALUE (link));
+	  if (t && DECL_P (t))
+	    note_addressable (t, s_ann);
+	}
+
+      get_expr_operands (stmt, &TREE_VALUE (link), opf_is_def, prev_vops);
+    }
+
+  for (link = ASM_INPUTS (stmt); link; link = TREE_CHAIN (link))
+    {
+      constraint
+	= TREE_STRING_POINTER (TREE_VALUE (TREE_PURPOSE (link)));
+      parse_input_constraint (&constraint, 0, 0, noutputs, 0,
+	  oconstraints, &allows_mem, &allows_reg);
+
+      /* Memory operands are addressable.  Note that STMT needs the
+	 address of this operand.  */
+      if (!allows_reg && allows_mem)
+	{
+	  tree t = get_base_address (TREE_VALUE (link));
+	  if (t && DECL_P (t))
+	    note_addressable (t, s_ann);
+	}
+
+      get_expr_operands (stmt, &TREE_VALUE (link), 0, prev_vops);
+    }
+
+  /* Clobber memory for asm ("" : : : "memory");  */
+  if (!aliases_computed_p)
+    {
+      /* If we still have not computed aliasing information,
+	 mark the statement as having volatile operands to avoid
+	 optimizations from messing around with it.  */
+      stmt_ann (stmt)->has_volatile_ops = true;
+    }
+  else
+    {
+      /* Otherwise, if this ASM_EXPR clobbers memory, clobber
+	 all the call-clobbered variables and the addressable
+	 variables found by the alias analyzer.  */
+      for (link = ASM_CLOBBERS (stmt); link; link = TREE_CHAIN (link))
+	if (!strcmp (TREE_STRING_POINTER (TREE_VALUE (link)), "memory"))
+	  {
+	    /* If we had created .GLOBAL_VAR earlier, use it.
+	       Otherwise, add a V_MAY_DEF operand for every
+	       call-clobbered and addressable variable.  See
+	       compute_may_aliases for the heuristic used to decide
+	       whether to create .GLOBAL_VAR or not.  */
+	    if (global_var)
+	      add_stmt_operand (&global_var, stmt, opf_is_def, prev_vops);
+	    else
+	      {
+		size_t i;
+
+		EXECUTE_IF_SET_IN_BITMAP (call_clobbered_vars, 0, i,
+		    {
+		      tree var = referenced_var (i);
+		      add_stmt_operand (&var, stmt, opf_is_def, prev_vops);
+		    });
+
+		EXECUTE_IF_SET_IN_BITMAP (addressable_vars, 0, i,
+		    {
+		      tree var = referenced_var (i);
+		      add_stmt_operand (&var, stmt, opf_is_def, prev_vops);
+		    });
+	      }
+	  }
+    }
+}
+
 
 /* Add *VAR_P to the appropriate operand array of STMT.  FLAGS is as in
    get_expr_operands.  If *VAR_P is a GIMPLE register, it will be added to
Index: tree-ssa.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/tree-ssa.c,v
retrieving revision 2.18
diff -d -c -p -d -u -p -r2.18 tree-ssa.c
--- tree-ssa.c	3 Jul 2004 13:45:28 -0000	2.18
+++ tree-ssa.c	8 Jul 2004 04:50:13 -0000
@@ -494,6 +494,7 @@ init_tree_ssa (void)
 {
   VARRAY_TREE_INIT (referenced_vars, 20, "referenced_vars");
   call_clobbered_vars = BITMAP_XMALLOC ();
+  addressable_vars = BITMAP_XMALLOC ();
   init_ssa_operands ();
   init_ssanames ();
   init_phinodes ();
@@ -532,6 +533,8 @@ delete_tree_ssa (void)
   BITMAP_XFREE (call_clobbered_vars);
   call_clobbered_vars = NULL;
   aliases_computed_p = false;
+  BITMAP_XFREE (addressable_vars);
+  addressable_vars = NULL;
 }

Re: Fix handling of ASM_EXPR operands

[Richard Henderson]

On Thu, Jul 08, 2004 at 02:29:19AM -0400, Diego Novillo wrote:
> +  /* Clobber memory for asm ("" : : : "memory");  */
> +  if (!aliases_computed_p)
> +    {
> +      /* If we still have not computed aliasing information,
> +	 mark the statement as having volatile operands to avoid
> +	 optimizations from messing around with it.  */
> +      stmt_ann (stmt)->has_volatile_ops = true;
> +    }
> +  else
> +    {
> +      /* Otherwise, if this ASM_EXPR clobbers memory, clobber
> +	 all the call-clobbered variables and the addressable
> +	 variables found by the alias analyzer.  */
> +      for (link = ASM_CLOBBERS (stmt); link; link = TREE_CHAIN (link))
> +	if (!strcmp (TREE_STRING_POINTER (TREE_VALUE (link)), "memory"))

You should move the for/if first.  No need to set has_volatile_ops
for an empty ASM_CLOBBERS list.

> +	    if (global_var)
> +	      add_stmt_operand (&global_var, stmt, opf_is_def, prev_vops);

Doesn't global_var only take care of call_clobbered_vars?


r~

Re: Fix handling of ASM_EXPR operands

[Diego Novillo]

On Thu, 2004-07-08 at 02:52, Richard Henderson wrote:

> > +	    if (global_var)
> > +	      add_stmt_operand (&global_var, stmt, opf_is_def, prev_vops);
> 
> Doesn't global_var only take care of call_clobbered_vars?
> 
Indeed.  How sloppy of me.  Thanks for pointing it out.  Here's the
right version.

Bootstrapped and tested x86, ppc and x86-64.


Diego.


	* tree-ssa-operands.c (get_asm_expr_operands): Fix thinkos in
	the handling of clobbering ASM_EXPRs.

Index: tree-ssa-operands.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/tree-ssa-operands.c,v
retrieving revision 2.14
diff -d -c -p -r2.14 tree-ssa-operands.c
*** tree-ssa-operands.c	8 Jul 2004 06:34:23 -0000	2.14
--- tree-ssa-operands.c	8 Jul 2004 12:11:13 -0000
*************** get_expr_operands (tree stmt, tree *expr
*** 1164,1171 ****
    abort ();
  }
  
! /* Scan operands in ASM_EXPR STMT.  PREV_VOPS is as in
!    append_v_may_def and append_vuse.  */
  
  static void
  get_asm_expr_operands (tree stmt, voperands_t prev_vops)
--- 1164,1172 ----
    abort ();
  }
  
! 
! /* Scan operands in ASM_EXPR STMT.  PREV_VOPS is as in append_v_may_def and
!    append_vuse.  */
  
  static void
  get_asm_expr_operands (tree stmt, voperands_t prev_vops)
*************** get_asm_expr_operands (tree stmt, vopera
*** 1223,1269 ****
        get_expr_operands (stmt, &TREE_VALUE (link), 0, prev_vops);
      }
  
    /* Clobber memory for asm ("" : : : "memory");  */
!   if (!aliases_computed_p)
!     {
!       /* If we still have not computed aliasing information,
! 	 mark the statement as having volatile operands to avoid
! 	 optimizations from messing around with it.  */
!       stmt_ann (stmt)->has_volatile_ops = true;
!     }
!   else
!     {
!       /* Otherwise, if this ASM_EXPR clobbers memory, clobber
! 	 all the call-clobbered variables and the addressable
! 	 variables found by the alias analyzer.  */
!       for (link = ASM_CLOBBERS (stmt); link; link = TREE_CHAIN (link))
! 	if (!strcmp (TREE_STRING_POINTER (TREE_VALUE (link)), "memory"))
  	  {
! 	    /* If we had created .GLOBAL_VAR earlier, use it.
! 	       Otherwise, add a V_MAY_DEF operand for every
! 	       call-clobbered and addressable variable.  See
! 	       compute_may_aliases for the heuristic used to decide
! 	       whether to create .GLOBAL_VAR or not.  */
! 	    if (global_var)
! 	      add_stmt_operand (&global_var, stmt, opf_is_def, prev_vops);
! 	    else
  	      {
! 		size_t i;
  
! 		EXECUTE_IF_SET_IN_BITMAP (call_clobbered_vars, 0, i,
! 		    {
! 		      tree var = referenced_var (i);
! 		      add_stmt_operand (&var, stmt, opf_is_def, prev_vops);
! 		    });
  
! 		EXECUTE_IF_SET_IN_BITMAP (addressable_vars, 0, i,
! 		    {
! 		      tree var = referenced_var (i);
! 		      add_stmt_operand (&var, stmt, opf_is_def, prev_vops);
! 		    });
! 	      }
! 	  }
!     }
  }
  
  
--- 1224,1266 ----
        get_expr_operands (stmt, &TREE_VALUE (link), 0, prev_vops);
      }
  
+ 
    /* Clobber memory for asm ("" : : : "memory");  */
!   for (link = ASM_CLOBBERS (stmt); link; link = TREE_CHAIN (link))
!     if (strcmp (TREE_STRING_POINTER (TREE_VALUE (link)), "memory") == 0)
!       {
! 	size_t i;
! 
! 	/* If we still have not computed aliasing information, we
! 	   won't know what variables are call-clobbered and/or
! 	   addressable.  Just mark the statement as having volatile
! 	   operands for now.  */
! 	if (!aliases_computed_p)
  	  {
! 	    stmt_ann (stmt)->has_volatile_ops = true;
! 	    break;
! 	  }
! 
! 	/* Clobber all call-clobbered variables (or .GLOBAL_VAR if we
! 	   decided to group them).  */
! 	if (global_var)
! 	  add_stmt_operand (&global_var, stmt, opf_is_def, prev_vops);
! 	else
! 	  EXECUTE_IF_SET_IN_BITMAP (call_clobbered_vars, 0, i,
  	      {
! 		tree var = referenced_var (i);
! 		add_stmt_operand (&var, stmt, opf_is_def, prev_vops);
! 	      });
  
! 	/* Now clobber all addressables.  */
! 	EXECUTE_IF_SET_IN_BITMAP (addressable_vars, 0, i,
! 	    {
! 	      tree var = referenced_var (i);
! 	      add_stmt_operand (&var, stmt, opf_is_def, prev_vops);
! 	    });
  
! 	break;
!       }
  }
  
  

Re: Fix handling of ASM_EXPR operands

[Richard Henderson]

On Thu, Jul 08, 2004 at 11:04:40AM -0400, Diego Novillo wrote:
> ! 	/* Clobber all call-clobbered variables (or .GLOBAL_VAR if we
> ! 	   decided to group them).  */
> ! 	if (global_var)
> ! 	  add_stmt_operand (&global_var, stmt, opf_is_def, prev_vops);
> ! 	else
> ! 	  EXECUTE_IF_SET_IN_BITMAP (call_clobbered_vars, 0, i,
>   	      {
> ! 		tree var = referenced_var (i);
> ! 		add_stmt_operand (&var, stmt, opf_is_def, prev_vops);
> ! 	      });
>   
> ! 	/* Now clobber all addressables.  */
> ! 	EXECUTE_IF_SET_IN_BITMAP (addressable_vars, 0, i,
> ! 	    {
> ! 	      tree var = referenced_var (i);
> ! 	      add_stmt_operand (&var, stmt, opf_is_def, prev_vops);
> ! 	    });

Isn't addressable_vars a superset of call_clobbered_vars?
In which case I'd think you'd want

	if (global_var)
	  {
	    add_stmt_operand (&global_var, stmt, opf_is_def, prev_vops);

	    EXECUTE_IF_AND_COMPL_IN_BITMAP (addressable_vars,
		call_clobbered_vars, 0, i,
		{
		  tree var = referenced_var (i);
		  add_stmt_operand (&var, stmt, opf_is_def, prev_vops);
		});
	  }
	else
	  {
	    EXECUTE_IF_SET_IN_BITMAP (addressable_vars, 0, i,
	      {
		tree var = referenced_var (i);
		add_stmt_operand (&var, stmt, opf_is_def, prev_vops);
	      });
	  }


r~

Re: Fix handling of ASM_EXPR operands

[Diego Novillo]

On Thu, 2004-07-08 at 12:02, Richard Henderson wrote:

> Isn't addressable_vars a superset of call_clobbered_vars?
>
Nope.

foo (int i)
{
  static int c = 0;
  int x, *p;
  p = (i > 10) ? &x : &i;
}

'c' is call-clobbered but not addressable.
'x' and 'i' are addressable but not call-clobbered.


Diego.