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

[Richard Guenther <rguenther at suse dot de>]

On Tue, 18 Mar 2008, Richard Guenther wrote:

> On Mon, 17 Mar 2008, Diego Novillo wrote:
> 
> > On 03/17/08 10:46, Richard Guenther wrote:
> > 
> > > 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).
> > 
> > You mean a caching mechanism for is_gimple_invariant_address?  I guess, but
> > first we'd have to find a significant slowdown with your patch. Caching
> > invariantness is what brought us to this point.
> 
> Yes, I mean caching only of the leaf part, that is
> 
>   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;
> 
> but - looking at the implementation of staticp we should better inline it
> (it contains lots of irrelevant code for the case we use it).
> 
> > > 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.
> > 
> > Looks OK now.
> 
> Bootstrapped and tested ok, I'll inline staticp here and re-do
> the testing.
> 
> Ok if that passes?

Bootstrapped and tested on x86_64-unknown-linux-gnu, ok for trunk?

(gfortran.fortran-torture/execute/der_init.f90 ICEs because we
build an address of an SSA_NAME during data dependency called from
predcom - I have a patch for this, referenced below in the tree-data-ref.c
part but yet untested)

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.

	* gcc.dg/tree-ssa/loop-19.c: 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,280 ----
      }
  }
  
+ /* 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 (CONSTANT_CLASS_P (op))
+     return true;
+ 
+   if (INDIRECT_REF_P (op))
+     return false;
+ 
+   switch (TREE_CODE (op))
+     {
+     case PARM_DECL:
+     case RESULT_DECL:
+     case LABEL_DECL:
+     case FUNCTION_DECL:
+       return true;
+ 
+     case VAR_DECL:
+       if (((TREE_STATIC (op) || DECL_EXTERNAL (op))
+ 	   && ! DECL_DLLIMPORT_P (op))
+ 	  || DECL_THREAD_LOCAL_P (op)
+ 	  || DECL_CONTEXT (op) == current_function_decl
+ 	  || decl_function_context (op) == current_function_decl)
+ 	return true;
+       break;
+ 
+     case CONST_DECL:
+       if ((TREE_STATIC (op) || DECL_EXTERNAL (op))
+ 	  || decl_function_context (op) == current_function_decl)
+ 	return true;
+       break;
+ 
+     default:
+       gcc_unreachable ();
+     }
+ 
+   return false;
+ }
+ 
+ /* 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
Index: gcc/testsuite/gcc.dg/tree-ssa/loop-19.c
===================================================================
*** gcc/testsuite/gcc.dg/tree-ssa/loop-19.c	(revision 133290)
--- gcc/testsuite/gcc.dg/tree-ssa/loop-19.c	(working copy)
***************
*** 6,12 ****
  
  /* { dg-do compile { target i?86-*-* x86_64-*-* powerpc*-*-*} } */
  /* { dg-require-effective-target nonpic } */
! /* { dg-options "-O2 -fdump-tree-final_cleanup" } */
  
  # define N      2000000
  static double   a[N],c[N];
--- 6,12 ----
  
  /* { dg-do compile { target i?86-*-* x86_64-*-* powerpc*-*-*} } */
  /* { dg-require-effective-target nonpic } */
! /* { dg-options "-O3 -fdump-tree-final_cleanup" } */
  
  # define N      2000000
  static double   a[N],c[N];


Index: tree-data-ref.c
===================================================================
--- tree-data-ref.c	(revision 133290)
+++ tree-data-ref.c	(working copy)
@@ -3965,11 +3965,14 @@ get_references_in_stmt (tree stmt, VEC (
 
   if (TREE_CODE (stmt) ==  GIMPLE_MODIFY_STMT)
     {
+      tree base;
       op0 = &GIMPLE_STMT_OPERAND (stmt, 0);
       op1 = &GIMPLE_STMT_OPERAND (stmt, 1);
 		
       if (DECL_P (*op1)
-	  || (REFERENCE_CLASS_P (*op1) && get_base_address (*op1)))
+	  || (REFERENCE_CLASS_P (*op1)
+	      && (base = get_base_address (*op1))
+	      && TREE_CODE (base) != SSA_NAME))
 	{
 	  ref = VEC_safe_push (data_ref_loc, heap, *references, NULL);
 	  ref->pos = op1;