Re: [PATCH] Fix PR35607, another revenge of invariant addresses (ivopts)

[Richard Guenther <rguenther at suse dot de>]

On Mon, 17 Mar 2008, Diego Novillo wrote:

> On 03/17/08 09:27, Richard Guenther wrote:
> 
> > + /* Return true if T is a gimple invariant address.  */
> > + + bool
> > + is_gimple_invariant_address (const_tree t)
> > + {
> > +   tree op;
> > + +   if (TREE_CODE (t) != ADDR_EXPR)
> > +     return false;
> > + +   op = TREE_OPERAND (t, 0);
> > +   while (handled_component_p (t))
> > +     {
> > +       switch (TREE_CODE (t))
> > + 	{
> > + 	case ARRAY_REF:
> > + 	case ARRAY_RANGE_REF:
> > + 	  if (!is_gimple_constant (TREE_OPERAND (t, 1))
> > + 	      || TREE_OPERAND (t, 2) != NULL_TREE
> > + 	      || TREE_OPERAND (t, 3) != NULL_TREE)
> > + 	    return false;
> > + 	  break;
> > + + 	case COMPONENT_REF:
> > + 	  if (TREE_OPERAND (t, 2) != NULL_TREE
> > + 	      || TREE_OPERAND (t, 3) != NULL_TREE)
> > + 	    return false;
> > + 	  break;
> > + + 	default:;
> > + 	}
> > +       op = TREE_OPERAND (t, 0);
> > +     }
> > + +   return TREE_INVARIANT (op);
> 
> This looks wrong.  It will return false for '&a' where 'a' is a local
> variable.  I think I would rather move the logic from
> recompute_tree_invariant_for_addr_expr.  This needs to handle statics and TLS
> objects.

Right.  I'm giving the following the honors of testing.  If that works
out we can throw it on some compile-time testing and after that get
rid of all calls to recompute_tree_invariant_for_addr_expr.  I also
see some uses of TREE_INVARIANT in DOM, ivopts, PRE and SCEV -- we
need to check them as well.  Otherwise TREE_INVARIANT is suspiciously
unused and we might be able to get rid of it completely (and maybe
replace it by marking possibly-invariant address DECLs to avoid
the costly checks there).

Thanks,
Richard.

2008-03-17  Richard Guenther  <rguenther@suse.de>

	* tree-gimple.h (is_gimple_invariant_address): Declare.
	(is_gimple_constant): Likewise.
	* tree-gimple.c (is_gimple_constant): New function.
	(is_gimple_invariant_address): Likewise.
	(is_gimple_min_invariant): Implement in terms of is_gimple_constant
	and is_gimple_invariant_address.
	* tree-ssa-loop-niter.c (expand_simple_operations): Revert
	previous change.

Index: gcc/tree-ssa-loop-niter.c
===================================================================
*** gcc/tree-ssa-loop-niter.c	(revision 133290)
--- gcc/tree-ssa-loop-niter.c	(working copy)
*************** expand_simple_operations (tree expr)
*** 1436,1445 ****
      return expr;
  
    e = GIMPLE_STMT_OPERAND (stmt, 1);
-   /* Do not expand random invariants.  */
-   if (TREE_INVARIANT (e)
-       && !is_gimple_min_invariant (e))
-     return expr;
    if (/* Casts are simple.  */
        TREE_CODE (e) != NOP_EXPR
        && TREE_CODE (e) != CONVERT_EXPR
--- 1436,1441 ----
Index: gcc/tree-gimple.h
===================================================================
*** gcc/tree-gimple.h	(revision 133290)
--- gcc/tree-gimple.h	(working copy)
*************** extern bool is_gimple_addressable (tree)
*** 62,67 ****
--- 62,71 ----
  /* Returns true iff T is any valid GIMPLE lvalue.  */
  extern bool is_gimple_lvalue (tree);
  
+ /* Returns true iff T is a GIMPLE invariant address.  */
+ bool is_gimple_invariant_address (const_tree);
+ /* Returns true iff T is a valid GIMPLE constant.  */
+ bool is_gimple_constant (const_tree);
  /* Returns true iff T is a GIMPLE restricted function invariant.  */
  extern bool is_gimple_min_invariant (const_tree);
  /* Returns true iff T is a GIMPLE rvalue.  */
Index: gcc/tree-gimple.c
===================================================================
*** gcc/tree-gimple.c	(revision 133290)
--- gcc/tree-gimple.c	(working copy)
*************** is_gimple_addressable (tree t)
*** 167,183 ****
  	  || INDIRECT_REF_P (t));
  }
  
! /* Return true if T is a GIMPLE minimal invariant.  It's a restricted
!    form of function invariant.  */
  
  bool
! is_gimple_min_invariant (const_tree t)
  {
    switch (TREE_CODE (t))
      {
-     case ADDR_EXPR:
-       return TREE_INVARIANT (t);
- 
      case INTEGER_CST:
      case REAL_CST:
      case FIXED_CST:
--- 167,179 ----
  	  || INDIRECT_REF_P (t));
  }
  
! /* Return true if T is a valid gimple constant.  */
  
  bool
! is_gimple_constant (const_tree t)
  {
    switch (TREE_CODE (t))
      {
      case INTEGER_CST:
      case REAL_CST:
      case FIXED_CST:
*************** is_gimple_min_invariant (const_tree t)
*** 198,203 ****
--- 194,254 ----
      }
  }
  
+ /* Return true if T is a gimple invariant address.  */
+ 
+ bool
+ is_gimple_invariant_address (const_tree t)
+ {
+   tree op;
+ 
+   if (TREE_CODE (t) != ADDR_EXPR)
+     return false;
+ 
+   op = TREE_OPERAND (t, 0);
+   while (handled_component_p (op))
+     {
+       switch (TREE_CODE (op))
+ 	{
+ 	case ARRAY_REF:
+ 	case ARRAY_RANGE_REF:
+ 	  if (!is_gimple_constant (TREE_OPERAND (op, 1))
+ 	      || TREE_OPERAND (op, 2) != NULL_TREE
+ 	      || TREE_OPERAND (op, 3) != NULL_TREE)
+ 	    return false;
+ 	  break;
+ 
+ 	case COMPONENT_REF:
+ 	  if (TREE_OPERAND (op, 2) != NULL_TREE)
+ 	    return false;
+ 	  break;
+ 
+ 	default:;
+ 	}
+       op = TREE_OPERAND (op, 0);
+     }
+ 
+   if (DECL_P (op)
+       && (staticp (op)
+ 	  || decl_function_context (op) == current_function_decl
+ 	  || (TREE_CODE (op) == VAR_DECL
+ 	      && DECL_THREAD_LOCAL_P (op))))
+     return true;
+ 
+   return CONSTANT_CLASS_P (op);
+ }
+ 
+ /* Return true if T is a GIMPLE minimal invariant.  It's a restricted
+    form of function invariant.  */
+ 
+ bool
+ is_gimple_min_invariant (const_tree t)
+ {
+   if (TREE_CODE (t) == ADDR_EXPR)
+     return is_gimple_invariant_address (t);
+ 
+   return is_gimple_constant (t);
+ }
+ 
  /* Return true if T looks like a valid GIMPLE statement.  */
  
  bool