Patches ping

Patches ping

[Zdenek Dvorak]

Hello,

first some really old patches:

http://gcc.gnu.org/ml/gcc-patches/2005-02/msg00205.html
  -- tree level store motion speed up (PR 17790)

http://gcc.gnu.org/ml/gcc-patches/2005-01/msg00796.html
  -- better heuristics for complete loop unrolling (PR 19401)

http://gcc.gnu.org/ml/gcc-patches/2005-01/msg01644.html
  -- the patch to include ggc memory consumption report
     to the standard timevar report

and from the last month:

http://gcc.gnu.org/ml/gcc-patches/2005-04/msg01810.html
  -- the patch to verify that statements operands are up-to-date

Zdenek

Re: Patches ping

[Andrew MacLeod]

On Wed, 2005-05-04 at 07:37, Zdenek Dvorak wrote:
> Hello,

> and from the last month:
> 
> http://gcc.gnu.org/ml/gcc-patches/2005-04/msg01810.html
>   -- the patch to verify that statements operands are up-to-date

Sorry, I forgot about this one.  The underlying implementation is now
completely different, I will update the tree-ssa-operands part of your
patch today.

Andrew 

Re: Patches ping

[Andrew MacLeod]

[-- Attachment #1: Type: text/plain, Size: 3918 bytes --]

On Wed, 2005-05-04 at 08:47, Andrew MacLeod wrote:
> On Wed, 2005-05-04 at 07:37, Zdenek Dvorak wrote:
> > Hello,
> 
> > and from the last month:
> > 
> > http://gcc.gnu.org/ml/gcc-patches/2005-04/msg01810.html
> >   -- the patch to verify that statements operands are up-to-date
> 
> Sorry, I forgot about this one.  The underlying implementation is now
> completely different, I will update the tree-ssa-operands part of your
> patch today.

Remind me, didn't you say when you first implemented this that it showed
a bunch of failures in the vectorizer?  I seem to recall that.

I am seeing some.  for instance:

gen-vect-11.c  

before updating:

#   VUSE <ib_57>;
vect_var_.6_1 = *vect_pib.7_2;

And after updating:

#   VUSE <ia>;
#   VUSE <ic>;
#   VUSE <ib_57>;
vect_var_.6_1 = *vect_pib.7_2;

Is this the behaviour you saw?  we will have some extra testsuite
failures until the vectorizer is fixed if I check this in until someone
looks at them.   The new failures I get are listed at the end.

Patch is attached. It bootstraps on i686-pc-linux-gnu.

I presume other people want to see this checked in even with the
additional testsuite failures? They are all with -ftree-vectorize. Or we
can hold off until the vectorizer is fixed.

Andrew

< FAIL: gcc.dg/tree-ssa/gen-vect-11.c (test for excess errors)
< FAIL: gcc.dg/tree-ssa/gen-vect-11a.c (test for excess errors)
< FAIL: gcc.dg/tree-ssa/gen-vect-2.c (test for excess errors)
< FAIL: gcc.dg/tree-ssa/gen-vect-25.c (test for excess errors)
< FAIL: gcc.dg/tree-ssa/ltrans-1.c (test for excess errors)
< FAIL: gcc.dg/vect/pr18400.c (test for excess errors)
< FAIL: gcc.dg/vect/pr20122.c (test for excess errors)
< FAIL: gcc.dg/vect/pr20122.c scan-tree-dump-times vectorized 1 loops 2
< FAIL: gcc.dg/vect/vect-1.c (test for excess errors)
< FAIL: gcc.dg/vect/vect-17.c (test for excess errors)
< FAIL: gcc.dg/vect/vect-18.c (test for excess errors)
< FAIL: gcc.dg/vect/vect-19.c (test for excess errors)
< FAIL: gcc.dg/vect/vect-2.c (test for excess errors)
< FAIL: gcc.dg/vect/vect-20.c (test for excess errors)
< FAIL: gcc.dg/vect/vect-22.c (test for excess errors)
< FAIL: gcc.dg/vect/vect-25.c (test for excess errors)
< FAIL: gcc.dg/vect/vect-27.c (test for excess errors)
< FAIL: gcc.dg/vect/vect-29.c (test for excess errors)
< FAIL: gcc.dg/vect/vect-3.c (test for excess errors)
< FAIL: gcc.dg/vect/vect-30.c (test for excess errors)
< FAIL: gcc.dg/vect/vect-30.c scan-tree-dump-times vectorized 1 loops 2
< FAIL: gcc.dg/vect/vect-34.c (test for excess errors)
< FAIL: gcc.dg/vect/vect-38.c (test for excess errors)
< FAIL: gcc.dg/vect/vect-4.c (test for excess errors)
< FAIL: gcc.dg/vect/vect-5.c (test for excess errors)
< FAIL: gcc.dg/vect/vect-6.c (test for excess errors)
< FAIL: gcc.dg/vect/vect-65.c (test for excess errors)
< FAIL: gcc.dg/vect/vect-66.c (test for excess errors)
< FAIL: gcc.dg/vect/vect-69.c (test for excess errors)
< FAIL: gcc.dg/vect/vect-7.c (test for excess errors)
< FAIL: gcc.dg/vect/vect-72.c (test for excess errors)
< FAIL: gcc.dg/vect/vect-8.c (test for excess errors)
< FAIL: gcc.dg/vect/vect-93.c (test for excess errors)
< FAIL: gcc.dg/vect/vect-96.c (test for excess errors)
< FAIL: gcc.dg/vect/vect-97.c (test for excess errors)
< FAIL: gcc.dg/vect/vect-all.c (test for excess errors)
< FAIL: gcc.dg/vect/vect-dv-2.c (test for excess errors)
< FAIL: gcc.dg/vect/vect-ifcvt-1.c (test for excess errors)
< FAIL: gcc.dg/vect/vect-none.c (test for excess errors)

< # of expected passes		35333
< # of unexpected failures	41
---
> # of expected passes		35406
> # of unexpected failures	2

< FAIL: gfortran.dg/vect/vect-1.f90 (test for excess errors)
< FAIL: gfortran.dg/vect/vect-2.f90 (test for excess errors)
< FAIL: gfortran.dg/vect/vect-5.f90 (test for excess errors)
7309,7310c7309,7310
< # of expected passes		6647
< # of unexpected failures	4
---
> # of expected passes		6651
> # of unexpected failures	1



[-- Attachment #2: check.diff --]
[-- Type: text/plain, Size: 8301 bytes --]

This implements Zdenek's stmt verification routine.  With the new 
implementation, all virtual operands are sorted, and reals are in 
order of appearence, so we can simply loop over the operands and 
stash them into a vector, update the stmt, and compare the results 
of a new itreation over the stmt with the old one.

Of course there is a hitch. get_expr_operands is capable of swapping 
a pair of real operand, so we have to add a hack to look for this 
situation where a swap has taken place. ick. 

2005-05-04  Andrew MacLeod  <amacleod@redhat.com>
	    Zdenek Dvorak  <dvorakz@suse.cz>

	* tree-ssa-operands.c (stmt_verify_vec): New.  Persistent vector for
	verify_stmt_operands to use.
	(init_ssa_operands): Assert that stmt_verify_vec is NULL.
	(fini_ssa_operands): Free stmt_verify_vec if necessary.
	(verify_stmt_operands): New.  Verify operands are up to date.
	* tree-ssa-operands.h (verify_stmt_operands): Declare prototype.
	* tree-ssa.c (verify_ssa): call verify_stmt_operands.


Index: tree-ssa-operands.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/tree-ssa-operands.c,v
retrieving revision 2.80
diff -c -p -r2.80 tree-ssa-operands.c
*** tree-ssa-operands.c	3 May 2005 12:19:46 -0000	2.80
--- tree-ssa-operands.c	4 May 2005 15:32:19 -0000
*************** static vuse_optype_p free_vuses = NULL;
*** 164,169 ****
--- 164,171 ----
  static maydef_optype_p free_maydefs = NULL;
  static mustdef_optype_p free_mustdefs = NULL;
  
+ static VEC(tree, heap) *stmt_verify_vec = NULL;
+ 
  /* Initialize a virtual operand build LIST called NAME with NUM elements.  */
  
  static inline void
*************** init_ssa_operands (void)
*** 401,406 ****
--- 403,409 ----
    opbuild_initialize_virtual (&build_v_may_defs, 25, "build_v_may_defs");
    opbuild_initialize_virtual (&build_v_must_defs, 25, "build_v_must_defs");
    gcc_assert (operand_memory == NULL);
+   gcc_assert (stmt_verify_vec == NULL);
    operand_memory_index = SSA_OPERAND_MEMORY_SIZE;
    ops_active = true;
  }
*************** fini_ssa_operands (void)
*** 440,445 ****
--- 443,450 ----
        ggc_free (ro_call_vuses);
        ro_call_vuses = NULL;
      }
+   if (stmt_verify_vec)
+     VEC_free (tree, heap, stmt_verify_vec);
    ops_active = false;
  }
  
*************** finalize_ssa_uses (tree stmt)
*** 579,585 ****
  
      /* If the pointer to the operand is the statement itself, something is
         wrong.  It means that we are pointing to a local variable (the 
!        initial call to get_stmt_operands does not pass a pointer to a 
         statement).  */
      for (x = 0; x < num; x++)
        gcc_assert (*(opbuild_elem_real (&build_uses, x)) != stmt);
--- 584,590 ----
  
      /* If the pointer to the operand is the statement itself, something is
         wrong.  It means that we are pointing to a local variable (the 
!        initial call to update_stmt_operands does not pass a pointer to a 
         statement).  */
      for (x = 0; x < num; x++)
        gcc_assert (*(opbuild_elem_real (&build_uses, x)) != stmt);
*************** build_ssa_operands (tree stmt)
*** 994,1056 ****
      }
  
    start_ssa_stmt_operands ();
- 
    parse_ssa_operands (stmt);
- 
    finalize_ssa_stmt_operands (stmt);
  }
  
  
! /* Free any operands vectors in OPS.  */
! #if 0
! static void 
! free_ssa_operands (stmt_operands_p ops)
! {
!   ops->def_ops = NULL;
!   ops->use_ops = NULL;
!   ops->maydef_ops = NULL;
!   ops->mustdef_ops = NULL;
!   ops->vuse_ops = NULL;
!   while (ops->memory.next != NULL)
!     {
!       operand_memory_p tmp = ops->memory.next;
!       ops->memory.next = tmp->next;
!       ggc_free (tmp);
!     }
! }
! #endif
! 
! 
! /* Get the operands of statement STMT.  Note that repeated calls to
!    get_stmt_operands for the same statement will do nothing until the
!    statement is marked modified by a call to mark_stmt_modified().  */
  
  void
  update_stmt_operands (tree stmt)
  {
    stmt_ann_t ann = get_stmt_ann (stmt);
!   /* If get_stmt_operands is called before SSA is initialized, dont
    do anything.  */
    if (!ssa_operands_active ())
      return;
    /* The optimizers cannot handle statements that are nothing but a
       _DECL.  This indicates a bug in the gimplifier.  */
    gcc_assert (!SSA_VAR_P (stmt));
- 
    gcc_assert (ann->modified);
  
    timevar_push (TV_TREE_OPS);
- 
    build_ssa_operands (stmt);
  
!   /* Clear the modified bit for STMT.  Subsequent calls to
!      get_stmt_operands for this statement will do nothing until the
!      statement is marked modified by a call to mark_stmt_modified().  */
    ann->modified = 0;
- 
    timevar_pop (TV_TREE_OPS);
  }
  
    
  /* Copies virtual operands from SRC to DST.  */
  
--- 999,1083 ----
      }
  
    start_ssa_stmt_operands ();
    parse_ssa_operands (stmt);
    finalize_ssa_stmt_operands (stmt);
  }
  
  
! /* Get the operands of statement STMT.  */
  
  void
  update_stmt_operands (tree stmt)
  {
    stmt_ann_t ann = get_stmt_ann (stmt);
!   /* If update_stmt_operands is called before SSA is initialized, dont
    do anything.  */
    if (!ssa_operands_active ())
      return;
    /* The optimizers cannot handle statements that are nothing but a
       _DECL.  This indicates a bug in the gimplifier.  */
    gcc_assert (!SSA_VAR_P (stmt));
    gcc_assert (ann->modified);
  
    timevar_push (TV_TREE_OPS);
    build_ssa_operands (stmt);
  
!   /* Clear the modified bit for STMT.  */
    ann->modified = 0;
    timevar_pop (TV_TREE_OPS);
  }
  
+ 
+ /* Verify the operands are up to date in STMT.  */
+ 
+ void
+ verify_stmt_operands (tree stmt)
+ {
+   ssa_op_iter iter;
+   tree t, old;
+   unsigned i;
+   bool swapped = false;
+   
+   if (!ssa_operands_active ())
+     return;
+ 
+   if (!stmt_verify_vec)
+     stmt_verify_vec = VEC_alloc (tree, heap, 100);
+   else
+     VEC_truncate (tree, stmt_verify_vec, 0);
+ 
+   /* Operands are ordered, so simply stash them into a vector.  */
+   FOR_EACH_SSA_TREE_OPERAND (t, stmt, iter, SSA_OP_ALL_OPERANDS)
+     VEC_safe_push (tree, heap, stmt_verify_vec, t);
+ 
+   update_stmt (stmt);
+ 
+   i = 0;
+   /* Now compare each vector element to the rebuilt operands.  */
+   FOR_EACH_SSA_TREE_OPERAND (t, stmt, iter, SSA_OP_ALL_OPERANDS)
+     {
+       VEC_iterate (tree, stmt_verify_vec, i++, old);
+       /* get_expr_operands can swap real operands, so look for swapped
+ 	 operands.  */
+       if (swapped)
+         {
+ 	  gcc_assert (VEC_index (tree, stmt_verify_vec, i - 2) == t);
+ 	  swapped = false;
+ 	  continue;
+ 	}
+       if (old != t && i < VEC_length (tree, stmt_verify_vec)
+ 	  && VEC_index (tree, stmt_verify_vec, i) == t)
+ 	{
+ 	  swapped = true;
+ 	  continue;
+ 	}
+       /* OLD will be NULL and miscompare if the vector iteration ends early,  */
+       gcc_assert (old == t);
+     }
+ 
+   gcc_assert (i == VEC_length (tree, stmt_verify_vec));
+ }
+ 
    
  /* Copies virtual operands from SRC to DST.  */
  
Index: tree-ssa-operands.h
===================================================================
RCS file: /cvs/gcc/gcc/gcc/tree-ssa-operands.h,v
retrieving revision 2.15
diff -c -p -r2.15 tree-ssa-operands.h
*** tree-ssa-operands.h	3 May 2005 12:19:47 -0000	2.15
--- tree-ssa-operands.h	4 May 2005 15:32:19 -0000
*************** typedef struct stmt_operands_d *stmt_ope
*** 154,159 ****
--- 154,160 ----
  extern void init_ssa_operands (void);
  extern void fini_ssa_operands (void);
  extern void update_stmt_operands (tree);
+ extern void verify_stmt_operands (tree);
  extern bool verify_imm_links (FILE *f, tree var);
  
  extern void copy_virtual_operands (tree, tree);
Index: tree-ssa.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/tree-ssa.c,v
retrieving revision 2.93
diff -c -p -r2.93 tree-ssa.c
*** tree-ssa.c	3 May 2005 12:19:48 -0000	2.93
--- tree-ssa.c	4 May 2005 15:32:19 -0000
*************** verify_ssa (bool check_modified_stmt)
*** 755,760 ****
--- 755,763 ----
  
  	  FOR_EACH_SSA_TREE_OPERAND (op, stmt, iter, SSA_OP_ALL_DEFS)
  	    bitmap_set_bit (names_defined_in_bb, SSA_NAME_VERSION (op));
+ 
+ 	  /* Finally, make sure the operands are properly up to date.  */
+ 	  verify_stmt_operands (stmt);
  	}
  
        bitmap_clear (names_defined_in_bb);

Re: Patches ping

[Zdenek Dvorak]

Hello,

> > > and from the last month:
> > > 
> > > http://gcc.gnu.org/ml/gcc-patches/2005-04/msg01810.html
> > >   -- the patch to verify that statements operands are up-to-date
> > 
> > Sorry, I forgot about this one.  The underlying implementation is now
> > completely different, I will update the tree-ssa-operands part of your
> > patch today.
> 
> Remind me, didn't you say when you first implemented this that it showed
> a bunch of failures in the vectorizer?  I seem to recall that.

yes.

> Patch is attached. It bootstraps on i686-pc-linux-gnu.
> 
> I presume other people want to see this checked in even with the
> additional testsuite failures? They are all with -ftree-vectorize. Or we
> can hold off until the vectorizer is fixed.

The problem is known for about a month now, and nobody bothered to do
anything about it.  Perhaps making vectorizer ICE on basically any input will
persuade some of the vectorizer developers to finally fix it :-)

Zdenek

> < FAIL: gcc.dg/tree-ssa/gen-vect-11.c (test for excess errors)
> < FAIL: gcc.dg/tree-ssa/gen-vect-11a.c (test for excess errors)
> < FAIL: gcc.dg/tree-ssa/gen-vect-2.c (test for excess errors)
> < FAIL: gcc.dg/tree-ssa/gen-vect-25.c (test for excess errors)
> < FAIL: gcc.dg/tree-ssa/ltrans-1.c (test for excess errors)
> < FAIL: gcc.dg/vect/pr18400.c (test for excess errors)
> < FAIL: gcc.dg/vect/pr20122.c (test for excess errors)
> < FAIL: gcc.dg/vect/pr20122.c scan-tree-dump-times vectorized 1 loops 2
> < FAIL: gcc.dg/vect/vect-1.c (test for excess errors)
> < FAIL: gcc.dg/vect/vect-17.c (test for excess errors)
> < FAIL: gcc.dg/vect/vect-18.c (test for excess errors)
> < FAIL: gcc.dg/vect/vect-19.c (test for excess errors)
> < FAIL: gcc.dg/vect/vect-2.c (test for excess errors)
> < FAIL: gcc.dg/vect/vect-20.c (test for excess errors)
> < FAIL: gcc.dg/vect/vect-22.c (test for excess errors)
> < FAIL: gcc.dg/vect/vect-25.c (test for excess errors)
> < FAIL: gcc.dg/vect/vect-27.c (test for excess errors)
> < FAIL: gcc.dg/vect/vect-29.c (test for excess errors)
> < FAIL: gcc.dg/vect/vect-3.c (test for excess errors)
> < FAIL: gcc.dg/vect/vect-30.c (test for excess errors)
> < FAIL: gcc.dg/vect/vect-30.c scan-tree-dump-times vectorized 1 loops 2
> < FAIL: gcc.dg/vect/vect-34.c (test for excess errors)
> < FAIL: gcc.dg/vect/vect-38.c (test for excess errors)
> < FAIL: gcc.dg/vect/vect-4.c (test for excess errors)
> < FAIL: gcc.dg/vect/vect-5.c (test for excess errors)
> < FAIL: gcc.dg/vect/vect-6.c (test for excess errors)
> < FAIL: gcc.dg/vect/vect-65.c (test for excess errors)
> < FAIL: gcc.dg/vect/vect-66.c (test for excess errors)
> < FAIL: gcc.dg/vect/vect-69.c (test for excess errors)
> < FAIL: gcc.dg/vect/vect-7.c (test for excess errors)
> < FAIL: gcc.dg/vect/vect-72.c (test for excess errors)
> < FAIL: gcc.dg/vect/vect-8.c (test for excess errors)
> < FAIL: gcc.dg/vect/vect-93.c (test for excess errors)
> < FAIL: gcc.dg/vect/vect-96.c (test for excess errors)
> < FAIL: gcc.dg/vect/vect-97.c (test for excess errors)
> < FAIL: gcc.dg/vect/vect-all.c (test for excess errors)
> < FAIL: gcc.dg/vect/vect-dv-2.c (test for excess errors)
> < FAIL: gcc.dg/vect/vect-ifcvt-1.c (test for excess errors)
> < FAIL: gcc.dg/vect/vect-none.c (test for excess errors)
> 
> < # of expected passes		35333
> < # of unexpected failures	41
> ---
> > # of expected passes		35406
> > # of unexpected failures	2
> 
> < FAIL: gfortran.dg/vect/vect-1.f90 (test for excess errors)
> < FAIL: gfortran.dg/vect/vect-2.f90 (test for excess errors)
> < FAIL: gfortran.dg/vect/vect-5.f90 (test for excess errors)
> 7309,7310c7309,7310
> < # of expected passes		6647
> < # of unexpected failures	4
> ---
> > # of expected passes		6651
> > # of unexpected failures	1
> 
> 

> This implements Zdenek's stmt verification routine.  With the new 
> implementation, all virtual operands are sorted, and reals are in 
> order of appearence, so we can simply loop over the operands and 
> stash them into a vector, update the stmt, and compare the results 
> of a new itreation over the stmt with the old one.
> 
> Of course there is a hitch. get_expr_operands is capable of swapping 
> a pair of real operand, so we have to add a hack to look for this 
> situation where a swap has taken place. ick. 
> 
> 2005-05-04  Andrew MacLeod  <amacleod@redhat.com>
> 	    Zdenek Dvorak  <dvorakz@suse.cz>
> 
> 	* tree-ssa-operands.c (stmt_verify_vec): New.  Persistent vector for
> 	verify_stmt_operands to use.
> 	(init_ssa_operands): Assert that stmt_verify_vec is NULL.
> 	(fini_ssa_operands): Free stmt_verify_vec if necessary.
> 	(verify_stmt_operands): New.  Verify operands are up to date.
> 	* tree-ssa-operands.h (verify_stmt_operands): Declare prototype.
> 	* tree-ssa.c (verify_ssa): call verify_stmt_operands.
> 
> 
> Index: tree-ssa-operands.c
> ===================================================================
> RCS file: /cvs/gcc/gcc/gcc/tree-ssa-operands.c,v
> retrieving revision 2.80
> diff -c -p -r2.80 tree-ssa-operands.c
> *** tree-ssa-operands.c	3 May 2005 12:19:46 -0000	2.80
> --- tree-ssa-operands.c	4 May 2005 15:32:19 -0000
> *************** static vuse_optype_p free_vuses = NULL;
> *** 164,169 ****
> --- 164,171 ----
>   static maydef_optype_p free_maydefs = NULL;
>   static mustdef_optype_p free_mustdefs = NULL;
>   
> + static VEC(tree, heap) *stmt_verify_vec = NULL;
> + 
>   /* Initialize a virtual operand build LIST called NAME with NUM elements.  */
>   
>   static inline void
> *************** init_ssa_operands (void)
> *** 401,406 ****
> --- 403,409 ----
>     opbuild_initialize_virtual (&build_v_may_defs, 25, "build_v_may_defs");
>     opbuild_initialize_virtual (&build_v_must_defs, 25, "build_v_must_defs");
>     gcc_assert (operand_memory == NULL);
> +   gcc_assert (stmt_verify_vec == NULL);
>     operand_memory_index = SSA_OPERAND_MEMORY_SIZE;
>     ops_active = true;
>   }
> *************** fini_ssa_operands (void)
> *** 440,445 ****
> --- 443,450 ----
>         ggc_free (ro_call_vuses);
>         ro_call_vuses = NULL;
>       }
> +   if (stmt_verify_vec)
> +     VEC_free (tree, heap, stmt_verify_vec);
>     ops_active = false;
>   }
>   
> *************** finalize_ssa_uses (tree stmt)
> *** 579,585 ****
>   
>       /* If the pointer to the operand is the statement itself, something is
>          wrong.  It means that we are pointing to a local variable (the 
> !        initial call to get_stmt_operands does not pass a pointer to a 
>          statement).  */
>       for (x = 0; x < num; x++)
>         gcc_assert (*(opbuild_elem_real (&build_uses, x)) != stmt);
> --- 584,590 ----
>   
>       /* If the pointer to the operand is the statement itself, something is
>          wrong.  It means that we are pointing to a local variable (the 
> !        initial call to update_stmt_operands does not pass a pointer to a 
>          statement).  */
>       for (x = 0; x < num; x++)
>         gcc_assert (*(opbuild_elem_real (&build_uses, x)) != stmt);
> *************** build_ssa_operands (tree stmt)
> *** 994,1056 ****
>       }
>   
>     start_ssa_stmt_operands ();
> - 
>     parse_ssa_operands (stmt);
> - 
>     finalize_ssa_stmt_operands (stmt);
>   }
>   
>   
> ! /* Free any operands vectors in OPS.  */
> ! #if 0
> ! static void 
> ! free_ssa_operands (stmt_operands_p ops)
> ! {
> !   ops->def_ops = NULL;
> !   ops->use_ops = NULL;
> !   ops->maydef_ops = NULL;
> !   ops->mustdef_ops = NULL;
> !   ops->vuse_ops = NULL;
> !   while (ops->memory.next != NULL)
> !     {
> !       operand_memory_p tmp = ops->memory.next;
> !       ops->memory.next = tmp->next;
> !       ggc_free (tmp);
> !     }
> ! }
> ! #endif
> ! 
> ! 
> ! /* Get the operands of statement STMT.  Note that repeated calls to
> !    get_stmt_operands for the same statement will do nothing until the
> !    statement is marked modified by a call to mark_stmt_modified().  */
>   
>   void
>   update_stmt_operands (tree stmt)
>   {
>     stmt_ann_t ann = get_stmt_ann (stmt);
> !   /* If get_stmt_operands is called before SSA is initialized, dont
>     do anything.  */
>     if (!ssa_operands_active ())
>       return;
>     /* The optimizers cannot handle statements that are nothing but a
>        _DECL.  This indicates a bug in the gimplifier.  */
>     gcc_assert (!SSA_VAR_P (stmt));
> - 
>     gcc_assert (ann->modified);
>   
>     timevar_push (TV_TREE_OPS);
> - 
>     build_ssa_operands (stmt);
>   
> !   /* Clear the modified bit for STMT.  Subsequent calls to
> !      get_stmt_operands for this statement will do nothing until the
> !      statement is marked modified by a call to mark_stmt_modified().  */
>     ann->modified = 0;
> - 
>     timevar_pop (TV_TREE_OPS);
>   }
>   
>     
>   /* Copies virtual operands from SRC to DST.  */
>   
> --- 999,1083 ----
>       }
>   
>     start_ssa_stmt_operands ();
>     parse_ssa_operands (stmt);
>     finalize_ssa_stmt_operands (stmt);
>   }
>   
>   
> ! /* Get the operands of statement STMT.  */
>   
>   void
>   update_stmt_operands (tree stmt)
>   {
>     stmt_ann_t ann = get_stmt_ann (stmt);
> !   /* If update_stmt_operands is called before SSA is initialized, dont
>     do anything.  */
>     if (!ssa_operands_active ())
>       return;
>     /* The optimizers cannot handle statements that are nothing but a
>        _DECL.  This indicates a bug in the gimplifier.  */
>     gcc_assert (!SSA_VAR_P (stmt));
>     gcc_assert (ann->modified);
>   
>     timevar_push (TV_TREE_OPS);
>     build_ssa_operands (stmt);
>   
> !   /* Clear the modified bit for STMT.  */
>     ann->modified = 0;
>     timevar_pop (TV_TREE_OPS);
>   }
>   
> + 
> + /* Verify the operands are up to date in STMT.  */
> + 
> + void
> + verify_stmt_operands (tree stmt)
> + {
> +   ssa_op_iter iter;
> +   tree t, old;
> +   unsigned i;
> +   bool swapped = false;
> +   
> +   if (!ssa_operands_active ())
> +     return;
> + 
> +   if (!stmt_verify_vec)
> +     stmt_verify_vec = VEC_alloc (tree, heap, 100);
> +   else
> +     VEC_truncate (tree, stmt_verify_vec, 0);
> + 
> +   /* Operands are ordered, so simply stash them into a vector.  */
> +   FOR_EACH_SSA_TREE_OPERAND (t, stmt, iter, SSA_OP_ALL_OPERANDS)
> +     VEC_safe_push (tree, heap, stmt_verify_vec, t);
> + 
> +   update_stmt (stmt);
> + 
> +   i = 0;
> +   /* Now compare each vector element to the rebuilt operands.  */
> +   FOR_EACH_SSA_TREE_OPERAND (t, stmt, iter, SSA_OP_ALL_OPERANDS)
> +     {
> +       VEC_iterate (tree, stmt_verify_vec, i++, old);
> +       /* get_expr_operands can swap real operands, so look for swapped
> + 	 operands.  */
> +       if (swapped)
> +         {
> + 	  gcc_assert (VEC_index (tree, stmt_verify_vec, i - 2) == t);
> + 	  swapped = false;
> + 	  continue;
> + 	}
> +       if (old != t && i < VEC_length (tree, stmt_verify_vec)
> + 	  && VEC_index (tree, stmt_verify_vec, i) == t)
> + 	{
> + 	  swapped = true;
> + 	  continue;
> + 	}
> +       /* OLD will be NULL and miscompare if the vector iteration ends early,  */
> +       gcc_assert (old == t);
> +     }
> + 
> +   gcc_assert (i == VEC_length (tree, stmt_verify_vec));
> + }
> + 
>     
>   /* Copies virtual operands from SRC to DST.  */
>   
> Index: tree-ssa-operands.h
> ===================================================================
> RCS file: /cvs/gcc/gcc/gcc/tree-ssa-operands.h,v
> retrieving revision 2.15
> diff -c -p -r2.15 tree-ssa-operands.h
> *** tree-ssa-operands.h	3 May 2005 12:19:47 -0000	2.15
> --- tree-ssa-operands.h	4 May 2005 15:32:19 -0000
> *************** typedef struct stmt_operands_d *stmt_ope
> *** 154,159 ****
> --- 154,160 ----
>   extern void init_ssa_operands (void);
>   extern void fini_ssa_operands (void);
>   extern void update_stmt_operands (tree);
> + extern void verify_stmt_operands (tree);
>   extern bool verify_imm_links (FILE *f, tree var);
>   
>   extern void copy_virtual_operands (tree, tree);
> Index: tree-ssa.c
> ===================================================================
> RCS file: /cvs/gcc/gcc/gcc/tree-ssa.c,v
> retrieving revision 2.93
> diff -c -p -r2.93 tree-ssa.c
> *** tree-ssa.c	3 May 2005 12:19:48 -0000	2.93
> --- tree-ssa.c	4 May 2005 15:32:19 -0000
> *************** verify_ssa (bool check_modified_stmt)
> *** 755,760 ****
> --- 755,763 ----
>   
>   	  FOR_EACH_SSA_TREE_OPERAND (op, stmt, iter, SSA_OP_ALL_DEFS)
>   	    bitmap_set_bit (names_defined_in_bb, SSA_NAME_VERSION (op));
> + 
> + 	  /* Finally, make sure the operands are properly up to date.  */
> + 	  verify_stmt_operands (stmt);
>   	}
>   
>         bitmap_clear (names_defined_in_bb);

Re: Patches ping

[Devang Patel]

On May 4, 2005, at 11:44 AM, Zdenek Dvorak wrote:

> The problem is known for about a month now, and nobody bothered to do
> anything about it.

IIRC, one problem in this area was fixed By Diego and/or Jeff. In any  
case,
it would be useful if you provide more info to refresh our memory.

> Perhaps making vectorizer ICE on basically any input will
> persuade some of the vectorizer developers to finally fix it :-)

Definitely not a constructive suggestion.

-
Devang

Re: Patches ping

[Andrew MacLeod]

On Wed, 2005-05-04 at 15:23, Devang Patel wrote:
> On May 4, 2005, at 11:44 AM, Zdenek Dvorak wrote:
> 
> > The problem is known for about a month now, and nobody bothered to do
> > anything about it.
> 
> IIRC, one problem in this area was fixed By Diego and/or Jeff. In any  
> case,
> it would be useful if you provide more info to refresh our memory.

I don't know what the issue was. You can see it yourself if you apply
the patch. basically, at the end of the vectorizing pass, there are
statements which if you call update_stmt() on them, they get a different
set of operands. This means they needed to be updated during the pass
because they are out of date when the pass is over.

in the example I had in gen-vect-11.c  

before updating:

#   VUSE <ib_57>;
vect_var_.6_1 = *vect_pib.7_2;

And after updating:

#   VUSE <ia>;
#   VUSE <ic>;
#   VUSE <ib_57>;
vect_var_.6_1 = *vect_pib.7_2;

AS you can see, calling update_stmt on this stmt has added 2
additionally VUSES. I'm guessing this is alias related adn that the
alias sets for vect_pib.7_2 hgave changed, but that this stmt wasn't
updated after the alias sets were changed.

Just a guess tho.

the new code in the patch will simple assert/trap after it has updated
the statement to have the 3 uses, and detects that it doesn't match the
list from the 'before' case.  This all happens in verify_ssa.

Andrew

Re: Patches ping

[Zdenek Dvorak]

Hello,

> On Wed, 2005-05-04 at 15:23, Devang Patel wrote:
> > On May 4, 2005, at 11:44 AM, Zdenek Dvorak wrote:
> > 
> > > The problem is known for about a month now, and nobody bothered to do
> > > anything about it.
> > 
> > IIRC, one problem in this area was fixed By Diego and/or Jeff. In any  
> > case,
> > it would be useful if you provide more info to refresh our memory.
> 
> I don't know what the issue was. You can see it yourself if you apply
> the patch. basically, at the end of the vectorizing pass, there are
> statements which if you call update_stmt() on them, they get a different
> set of operands. This means they needed to be updated during the pass
> because they are out of date when the pass is over.
> 
> in the example I had in gen-vect-11.c  
> 
> before updating:
> 
> #   VUSE <ib_57>;
> vect_var_.6_1 = *vect_pib.7_2;
> 
> And after updating:
> 
> #   VUSE <ia>;
> #   VUSE <ic>;
> #   VUSE <ib_57>;
> vect_var_.6_1 = *vect_pib.7_2;
> 
> AS you can see, calling update_stmt on this stmt has added 2
> additionally VUSES. I'm guessing this is alias related adn that the
> alias sets for vect_pib.7_2 hgave changed, but that this stmt wasn't
> updated after the alias sets were changed.

the reason is that vect_pib does not have the aliasing info set
correctly.  The virtual operands the statement has should indeed
be just VUSE <ib_57>, but if I remember correctly, vect_pib.7_2
does not have name tag set and consequently is set to alias all
three arrays.

Zdenek

Re: Patches ping

[Devang Patel]

On May 4, 2005, at 1:25 PM, Andrew MacLeod wrote:

> On Wed, 2005-05-04 at 15:23, Devang Patel wrote:
>
>> On May 4, 2005, at 11:44 AM, Zdenek Dvorak wrote:
>>
>>
>>> The problem is known for about a month now, and nobody bothered  
>>> to do
>>> anything about it.
>>>
>>
>> IIRC, one problem in this area was fixed By Diego and/or Jeff. In any
>> case,
>> it would be useful if you provide more info to refresh our memory.
>>
>
> I don't know what the issue was. You can see it yourself if you apply
> the patch. basically, at the end of the vectorizing pass, there are
> statements which if you call update_stmt() on them, they get a  
> different
> set of operands. This means they needed to be updated during the pass
> because they are out of date when the pass is over.

I thought this was addressed by,
     http://gcc.gnu.org/ml/gcc/2005-04/msg00848.html

This thread started at
     Semi-Latent Bug in tree vectorizer
     http://gcc.gnu.org/ml/gcc/2005-04/msg00382.html

-
Devang

> in the example I had in gen-vect-11.c
>
> before updating:
>
> #   VUSE <ib_57>;
> vect_var_.6_1 = *vect_pib.7_2;
>
> And after updating:
>
> #   VUSE <ia>;
> #   VUSE <ic>;
> #   VUSE <ib_57>;
> vect_var_.6_1 = *vect_pib.7_2;
>
> AS you can see, calling update_stmt on this stmt has added 2
> additionally VUSES. I'm guessing this is alias related adn that the
> alias sets for vect_pib.7_2 hgave changed, but that this stmt wasn't
> updated after the alias sets were changed.
>
> Just a guess tho.
>
> the new code in the patch will simple assert/trap after it has updated
> the statement to have the 3 uses, and detects that it doesn't match  
> the
> list from the 'before' case.  This all happens in verify_ssa.
>
> Andrew
>
>

Re: Patches ping

[Zdenek Dvorak]

Hello,

> >>>The problem is known for about a month now, and nobody bothered  
> >>>to do
> >>>anything about it.
> >>>
> >>
> >>IIRC, one problem in this area was fixed By Diego and/or Jeff. In any
> >>case,
> >>it would be useful if you provide more info to refresh our memory.
> >>
> >
> >I don't know what the issue was. You can see it yourself if you apply
> >the patch. basically, at the end of the vectorizing pass, there are
> >statements which if you call update_stmt() on them, they get a  
> >different
> >set of operands. This means they needed to be updated during the pass
> >because they are out of date when the pass is over.
> 
> I thought this was addressed by,
>     http://gcc.gnu.org/ml/gcc/2005-04/msg00848.html
> 
> This thread started at
>     Semi-Latent Bug in tree vectorizer
>     http://gcc.gnu.org/ml/gcc/2005-04/msg00382.html

obviously, that was a different bug.

Zdenek

Re: Patches ping

[Devang Patel]

On May 4, 2005, at 2:07 PM, Zdenek Dvorak wrote:

> Hello,
>
>
>>>>> The problem is known for about a month now, and nobody bothered
>>>>> to do
>>>>> anything about it.
>>>>>
>>>>>
>>>>
>>>> IIRC, one problem in this area was fixed By Diego and/or Jeff.  
>>>> In any
>>>> case,
>>>> it would be useful if you provide more info to refresh our memory.
>>>>
>>>>
>>>
>>> I don't know what the issue was. You can see it yourself if you  
>>> apply
>>> the patch. basically, at the end of the vectorizing pass, there are
>>> statements which if you call update_stmt() on them, they get a
>>> different
>>> set of operands. This means they needed to be updated during the  
>>> pass
>>> because they are out of date when the pass is over.
>>>
>>
>> I thought this was addressed by,
>>     http://gcc.gnu.org/ml/gcc/2005-04/msg00848.html
>>
>> This thread started at
>>     Semi-Latent Bug in tree vectorizer
>>     http://gcc.gnu.org/ml/gcc/2005-04/msg00382.html
>>
>
> obviously, that was a different bug.

Well, I wanted to know what is "the known problem for about a month" ?
This was the only thing I knew and it looked similar. So, I appreciate
pointers for earlier discussions you referred above.

Otherwise, I'll try this patch myself, in next couple of days to
understand what is going on.

-
Devang

Re: Patches ping

[Zdenek Dvorak]

Hello,

> >>>I don't know what the issue was. You can see it yourself if you  
> >>>apply
> >>>the patch. basically, at the end of the vectorizing pass, there are
> >>>statements which if you call update_stmt() on them, they get a
> >>>different
> >>>set of operands. This means they needed to be updated during the  
> >>>pass
> >>>because they are out of date when the pass is over.
> >>>
> >>
> >>I thought this was addressed by,
> >>    http://gcc.gnu.org/ml/gcc/2005-04/msg00848.html
> >>
> >>This thread started at
> >>    Semi-Latent Bug in tree vectorizer
> >>    http://gcc.gnu.org/ml/gcc/2005-04/msg00382.html
> >>
> >
> >obviously, that was a different bug.
> 
> Well, I wanted to know what is "the known problem for about a month" ?
> This was the only thing I knew and it looked similar. So, I appreciate
> pointers for earlier discussions you referred above.

http://gcc.gnu.org/ml/gcc-patches/2005-04/msg01360.html

I did not really try to analyze the problem very much.  I just looked at
the problem, noticed that it is different from the one fixed by the
Jeff's patch (because it was not fixed by the patch and the symptoms are
quite different -- there are no "forgotten" virtual operands, just the
aliasing info on pointers is set up in a wrong way), and that I don't
immediatelly see where precisely is the problem in vectorizer (although
I suspect that the cause is that it somewhere just calls
copy_virtual_operands instead of updating the statement properly).

http://gcc.gnu.org/ml/gcc-patches/2005-04/msg01567.html

So I wrote the verifier and hoped that someone who understands the way
vectorizer is trying to update the virtual operands will fix that.

Zdenek

Re: Patches ping

[Dorit Naishlos]

>
> > Patch is attached. It bootstraps on i686-pc-linux-gnu.
> >
> > I presume other people want to see this checked in even with the
> > additional testsuite failures? They are all with -ftree-vectorize. Or
we
> > can hold off until the vectorizer is fixed.
>
> The problem is known for about a month now, and nobody bothered to do
> anything about it.  Perhaps making vectorizer ICE on basically any input
will
> persuade some of the vectorizer developers to finally fix it :-)
>

Keith has been looking into this. I believe he has a patch in the works

dorit


> Zdenek
>
> > < FAIL: gcc.dg/tree-ssa/gen-vect-11.c (test for excess errors)
> > < FAIL: gcc.dg/tree-ssa/gen-vect-11a.c (test for excess errors)
> > < FAIL: gcc.dg/tree-ssa/gen-vect-2.c (test for excess errors)
> > < FAIL: gcc.dg/tree-ssa/gen-vect-25.c (test for excess errors)
> > < FAIL: gcc.dg/tree-ssa/ltrans-1.c (test for excess errors)
> > < FAIL: gcc.dg/vect/pr18400.c (test for excess errors)
> > < FAIL: gcc.dg/vect/pr20122.c (test for excess errors)
> > < FAIL: gcc.dg/vect/pr20122.c scan-tree-dump-times vectorized 1 loops 2
> > < FAIL: gcc.dg/vect/vect-1.c (test for excess errors)
> > < FAIL: gcc.dg/vect/vect-17.c (test for excess errors)
> > < FAIL: gcc.dg/vect/vect-18.c (test for excess errors)
> > < FAIL: gcc.dg/vect/vect-19.c (test for excess errors)
> > < FAIL: gcc.dg/vect/vect-2.c (test for excess errors)
> > < FAIL: gcc.dg/vect/vect-20.c (test for excess errors)
> > < FAIL: gcc.dg/vect/vect-22.c (test for excess errors)
> > < FAIL: gcc.dg/vect/vect-25.c (test for excess errors)
> > < FAIL: gcc.dg/vect/vect-27.c (test for excess errors)
> > < FAIL: gcc.dg/vect/vect-29.c (test for excess errors)
> > < FAIL: gcc.dg/vect/vect-3.c (test for excess errors)
> > < FAIL: gcc.dg/vect/vect-30.c (test for excess errors)
> > < FAIL: gcc.dg/vect/vect-30.c scan-tree-dump-times vectorized 1 loops 2
> > < FAIL: gcc.dg/vect/vect-34.c (test for excess errors)
> > < FAIL: gcc.dg/vect/vect-38.c (test for excess errors)
> > < FAIL: gcc.dg/vect/vect-4.c (test for excess errors)
> > < FAIL: gcc.dg/vect/vect-5.c (test for excess errors)
> > < FAIL: gcc.dg/vect/vect-6.c (test for excess errors)
> > < FAIL: gcc.dg/vect/vect-65.c (test for excess errors)
> > < FAIL: gcc.dg/vect/vect-66.c (test for excess errors)
> > < FAIL: gcc.dg/vect/vect-69.c (test for excess errors)
> > < FAIL: gcc.dg/vect/vect-7.c (test for excess errors)
> > < FAIL: gcc.dg/vect/vect-72.c (test for excess errors)
> > < FAIL: gcc.dg/vect/vect-8.c (test for excess errors)
> > < FAIL: gcc.dg/vect/vect-93.c (test for excess errors)
> > < FAIL: gcc.dg/vect/vect-96.c (test for excess errors)
> > < FAIL: gcc.dg/vect/vect-97.c (test for excess errors)
> > < FAIL: gcc.dg/vect/vect-all.c (test for excess errors)
> > < FAIL: gcc.dg/vect/vect-dv-2.c (test for excess errors)
> > < FAIL: gcc.dg/vect/vect-ifcvt-1.c (test for excess errors)
> > < FAIL: gcc.dg/vect/vect-none.c (test for excess errors)
> >
> > < # of expected passes      35333
> > < # of unexpected failures   41
> > ---
> > > # of expected passes      35406
> > > # of unexpected failures   2
> >
> > < FAIL: gfortran.dg/vect/vect-1.f90 (test for excess errors)
> > < FAIL: gfortran.dg/vect/vect-2.f90 (test for excess errors)
> > < FAIL: gfortran.dg/vect/vect-5.f90 (test for excess errors)
> > 7309,7310c7309,7310
> > < # of expected passes      6647
> > < # of unexpected failures   4
> > ---
> > > # of expected passes      6651
> > > # of unexpected failures   1
> >
> >
>
> > This implements Zdenek's stmt verification routine.  With the new
> > implementation, all virtual operands are sorted, and reals are in
> > order of appearence, so we can simply loop over the operands and
> > stash them into a vector, update the stmt, and compare the results
> > of a new itreation over the stmt with the old one.
> >
> > Of course there is a hitch. get_expr_operands is capable of swapping
> > a pair of real operand, so we have to add a hack to look for this
> > situation where a swap has taken place. ick.
> >
> > 2005-05-04  Andrew MacLeod  <amacleod@redhat.com>
> >        Zdenek Dvorak  <dvorakz@suse.cz>
> >
> >    * tree-ssa-operands.c (stmt_verify_vec): New.  Persistent vector for
> >    verify_stmt_operands to use.
> >    (init_ssa_operands): Assert that stmt_verify_vec is NULL.
> >    (fini_ssa_operands): Free stmt_verify_vec if necessary.
> >    (verify_stmt_operands): New.  Verify operands are up to date.
> >    * tree-ssa-operands.h (verify_stmt_operands): Declare prototype.
> >    * tree-ssa.c (verify_ssa): call verify_stmt_operands.
> >
> >
> > Index: tree-ssa-operands.c
> > ===================================================================
> > RCS file: /cvs/gcc/gcc/gcc/tree-ssa-operands.c,v
> > retrieving revision 2.80
> > diff -c -p -r2.80 tree-ssa-operands.c
> > *** tree-ssa-operands.c   3 May 2005 12:19:46 -0000   2.80
> > --- tree-ssa-operands.c   4 May 2005 15:32:19 -0000
> > *************** static vuse_optype_p free_vuses = NULL;
> > *** 164,169 ****
> > --- 164,171 ----
> >   static maydef_optype_p free_maydefs = NULL;
> >   static mustdef_optype_p free_mustdefs = NULL;
> >
> > + static VEC(tree, heap) *stmt_verify_vec = NULL;
> > +
> >   /* Initialize a virtual operand build LIST called NAME with NUM
> elements.  */
> >
> >   static inline void
> > *************** init_ssa_operands (void)
> > *** 401,406 ****
> > --- 403,409 ----
> >     opbuild_initialize_virtual (&build_v_may_defs, 25,
"build_v_may_defs");
> >     opbuild_initialize_virtual (&build_v_must_defs, 25,
> "build_v_must_defs");
> >     gcc_assert (operand_memory == NULL);
> > +   gcc_assert (stmt_verify_vec == NULL);
> >     operand_memory_index = SSA_OPERAND_MEMORY_SIZE;
> >     ops_active = true;
> >   }
> > *************** fini_ssa_operands (void)
> > *** 440,445 ****
> > --- 443,450 ----
> >         ggc_free (ro_call_vuses);
> >         ro_call_vuses = NULL;
> >       }
> > +   if (stmt_verify_vec)
> > +     VEC_free (tree, heap, stmt_verify_vec);
> >     ops_active = false;
> >   }
> >
> > *************** finalize_ssa_uses (tree stmt)
> > *** 579,585 ****
> >
> >       /* If the pointer to the operand is the statement itself,
something is
> >          wrong.  It means that we are pointing to a local variable (the

> > !        initial call to get_stmt_operands does not pass a pointer to a

> >          statement).  */
> >       for (x = 0; x < num; x++)
> >         gcc_assert (*(opbuild_elem_real (&build_uses, x)) != stmt);
> > --- 584,590 ----
> >
> >       /* If the pointer to the operand is the statement itself,
something is
> >          wrong.  It means that we are pointing to a local variable (the

> > !        initial call to update_stmt_operands does not pass a pointer
to a
> >          statement).  */
> >       for (x = 0; x < num; x++)
> >         gcc_assert (*(opbuild_elem_real (&build_uses, x)) != stmt);
> > *************** build_ssa_operands (tree stmt)
> > *** 994,1056 ****
> >       }
> >
> >     start_ssa_stmt_operands ();
> > -
> >     parse_ssa_operands (stmt);
> > -
> >     finalize_ssa_stmt_operands (stmt);
> >   }
> >
> >
> > ! /* Free any operands vectors in OPS.  */
> > ! #if 0
> > ! static void
> > ! free_ssa_operands (stmt_operands_p ops)
> > ! {
> > !   ops->def_ops = NULL;
> > !   ops->use_ops = NULL;
> > !   ops->maydef_ops = NULL;
> > !   ops->mustdef_ops = NULL;
> > !   ops->vuse_ops = NULL;
> > !   while (ops->memory.next != NULL)
> > !     {
> > !       operand_memory_p tmp = ops->memory.next;
> > !       ops->memory.next = tmp->next;
> > !       ggc_free (tmp);
> > !     }
> > ! }
> > ! #endif
> > !
> > !
> > ! /* Get the operands of statement STMT.  Note that repeated calls to
> > !    get_stmt_operands for the same statement will do nothing until the
> > !    statement is marked modified by a call to mark_stmt_modified().
*/
> >
> >   void
> >   update_stmt_operands (tree stmt)
> >   {
> >     stmt_ann_t ann = get_stmt_ann (stmt);
> > !   /* If get_stmt_operands is called before SSA is initialized, dont
> >     do anything.  */
> >     if (!ssa_operands_active ())
> >       return;
> >     /* The optimizers cannot handle statements that are nothing but a
> >        _DECL.  This indicates a bug in the gimplifier.  */
> >     gcc_assert (!SSA_VAR_P (stmt));
> > -
> >     gcc_assert (ann->modified);
> >
> >     timevar_push (TV_TREE_OPS);
> > -
> >     build_ssa_operands (stmt);
> >
> > !   /* Clear the modified bit for STMT.  Subsequent calls to
> > !      get_stmt_operands for this statement will do nothing until the
> > !      statement is marked modified by a call to mark_stmt_modified().
*/
> >     ann->modified = 0;
> > -
> >     timevar_pop (TV_TREE_OPS);
> >   }
> >
> >
> >   /* Copies virtual operands from SRC to DST.  */
> >
> > --- 999,1083 ----
> >       }
> >
> >     start_ssa_stmt_operands ();
> >     parse_ssa_operands (stmt);
> >     finalize_ssa_stmt_operands (stmt);
> >   }
> >
> >
> > ! /* Get the operands of statement STMT.  */
> >
> >   void
> >   update_stmt_operands (tree stmt)
> >   {
> >     stmt_ann_t ann = get_stmt_ann (stmt);
> > !   /* If update_stmt_operands is called before SSA is initialized,
dont
> >     do anything.  */
> >     if (!ssa_operands_active ())
> >       return;
> >     /* The optimizers cannot handle statements that are nothing but a
> >        _DECL.  This indicates a bug in the gimplifier.  */
> >     gcc_assert (!SSA_VAR_P (stmt));
> >     gcc_assert (ann->modified);
> >
> >     timevar_push (TV_TREE_OPS);
> >     build_ssa_operands (stmt);
> >
> > !   /* Clear the modified bit for STMT.  */
> >     ann->modified = 0;
> >     timevar_pop (TV_TREE_OPS);
> >   }
> >
> > +
> > + /* Verify the operands are up to date in STMT.  */
> > +
> > + void
> > + verify_stmt_operands (tree stmt)
> > + {
> > +   ssa_op_iter iter;
> > +   tree t, old;
> > +   unsigned i;
> > +   bool swapped = false;
> > +
> > +   if (!ssa_operands_active ())
> > +     return;
> > +
> > +   if (!stmt_verify_vec)
> > +     stmt_verify_vec = VEC_alloc (tree, heap, 100);
> > +   else
> > +     VEC_truncate (tree, stmt_verify_vec, 0);
> > +
> > +   /* Operands are ordered, so simply stash them into a vector.  */
> > +   FOR_EACH_SSA_TREE_OPERAND (t, stmt, iter, SSA_OP_ALL_OPERANDS)
> > +     VEC_safe_push (tree, heap, stmt_verify_vec, t);
> > +
> > +   update_stmt (stmt);
> > +
> > +   i = 0;
> > +   /* Now compare each vector element to the rebuilt operands.  */
> > +   FOR_EACH_SSA_TREE_OPERAND (t, stmt, iter, SSA_OP_ALL_OPERANDS)
> > +     {
> > +       VEC_iterate (tree, stmt_verify_vec, i++, old);
> > +       /* get_expr_operands can swap real operands, so look for
swapped
> > +     operands.  */
> > +       if (swapped)
> > +         {
> > +      gcc_assert (VEC_index (tree, stmt_verify_vec, i - 2) == t);
> > +      swapped = false;
> > +      continue;
> > +    }
> > +       if (old != t && i < VEC_length (tree, stmt_verify_vec)
> > +      && VEC_index (tree, stmt_verify_vec, i) == t)
> > +    {
> > +      swapped = true;
> > +      continue;
> > +    }
> > +       /* OLD will be NULL and miscompare if the vector iteration
> ends early,  */
> > +       gcc_assert (old == t);
> > +     }
> > +
> > +   gcc_assert (i == VEC_length (tree, stmt_verify_vec));
> > + }
> > +
> >
> >   /* Copies virtual operands from SRC to DST.  */
> >
> > Index: tree-ssa-operands.h
> > ===================================================================
> > RCS file: /cvs/gcc/gcc/gcc/tree-ssa-operands.h,v
> > retrieving revision 2.15
> > diff -c -p -r2.15 tree-ssa-operands.h
> > *** tree-ssa-operands.h   3 May 2005 12:19:47 -0000   2.15
> > --- tree-ssa-operands.h   4 May 2005 15:32:19 -0000
> > *************** typedef struct stmt_operands_d *stmt_ope
> > *** 154,159 ****
> > --- 154,160 ----
> >   extern void init_ssa_operands (void);
> >   extern void fini_ssa_operands (void);
> >   extern void update_stmt_operands (tree);
> > + extern void verify_stmt_operands (tree);
> >   extern bool verify_imm_links (FILE *f, tree var);
> >
> >   extern void copy_virtual_operands (tree, tree);
> > Index: tree-ssa.c
> > ===================================================================
> > RCS file: /cvs/gcc/gcc/gcc/tree-ssa.c,v
> > retrieving revision 2.93
> > diff -c -p -r2.93 tree-ssa.c
> > *** tree-ssa.c   3 May 2005 12:19:48 -0000   2.93
> > --- tree-ssa.c   4 May 2005 15:32:19 -0000
> > *************** verify_ssa (bool check_modified_stmt)
> > *** 755,760 ****
> > --- 755,763 ----
> >
> >        FOR_EACH_SSA_TREE_OPERAND (op, stmt, iter, SSA_OP_ALL_DEFS)
> >          bitmap_set_bit (names_defined_in_bb, SSA_NAME_VERSION (op));
> > +
> > +      /* Finally, make sure the operands are properly up to date.  */
> > +      verify_stmt_operands (stmt);
> >      }
> >
> >         bitmap_clear (names_defined_in_bb);
>

Re: Patches ping

[Diego Novillo]

On Wed, May 04, 2005 at 01:37:58PM +0200, Zdenek Dvorak wrote:

> http://gcc.gnu.org/ml/gcc-patches/2005-02/msg00205.html
>   -- tree level store motion speed up (PR 17790)
> 
Send me an updated patch?  This one doesn't apply for me.


> http://gcc.gnu.org/ml/gcc-patches/2005-01/msg00796.html
>   -- better heuristics for complete loop unrolling (PR 19401)
> 
OK.  Though, could you factor the UL_NO_GROWTH estimation in
try_unroll_loop_completely into a separate helper?


> http://gcc.gnu.org/ml/gcc-patches/2005-01/msg01644.html
>   -- the patch to include ggc memory consumption report
>      to the standard timevar report
> 
This one looks very useful and I would like to see it go in.  But
I can't approve patches in this area.

I would only split the memory consumption hunks from the timevar
renames.


Thanks.  Diego.

Re: Patches ping

[Zdenek Dvorak]

Hello,

> > http://gcc.gnu.org/ml/gcc-patches/2005-02/msg00205.html
> >   -- tree level store motion speed up (PR 17790)
> > 
> Send me an updated patch?  This one doesn't apply for me.

here is the updated patch, bootstrapped & regtested on i686 and ia64.
PR17790 no longer reproduces for me (we spend no significant amount
of time in lim now); probably the problem got fixed or masked in
the meantime by some unrelated change.  Still, I like the new
implementation more than the old one (it gets the same results,
but I find it easier to read, and it also is by some 100 lines shorter).

Zdenek

	* tree-ssa-loop-im.c: Include hashtab.h.
	(struct mem_ref_loc): New.
	(struct mem_ref): Describe the set of references with the same
	shape.
	(max_stmt_uid, get_stmt_uid, record_mem_ref, free_mem_refs,
	maybe_queue_var, fem_single_reachable_address,
	for_each_memref, single_reachable_address,
	is_call_clobbered_ref, determine_lsm_reg): Removed.
	(record_mem_ref_loc, free_mem_ref_locs, determine_lsm_ref,
	hoist_memory_reference, memref_hash, memref_eq, memref_del,
	gather_mem_refs_stmt, gather_mem_refs, find_more_ref_vops):
	New functions.
	(rewrite_mem_refs, schedule_sm): Use mem_ref_loc list.
	(determine_lsm_loop): Rewritten.
	(determine_lsm): Do not set stmt uids.

Index: tree-ssa-loop-im.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/tree-ssa-loop-im.c,v
retrieving revision 2.39
diff -c -3 -p -r2.39 tree-ssa-loop-im.c
*** tree-ssa-loop-im.c	3 May 2005 20:18:32 -0000	2.39
--- tree-ssa-loop-im.c	5 May 2005 20:27:27 -0000
*************** Software Foundation, 59 Temple Place - S
*** 38,43 ****
--- 38,44 ----
  #include "tree-pass.h"
  #include "flags.h"
  #include "real.h"
+ #include "hashtab.h"
  
  /* TODO:  Support for predicated code motion.  I.e.
  
*************** struct lim_aux_data
*** 103,115 ****
  			? NULL \
  			: (struct lim_aux_data *) (stmt_ann (STMT)->aux))
  
! /* Description of a memory reference for store motion.  */
  
! struct mem_ref
  {
    tree *ref;			/* The reference itself.  */
    tree stmt;			/* The statement in that it occurs.  */
!   struct mem_ref *next;		/* Next use in the chain.  */
  };
  
  /* Minimum cost of an expensive expression.  */
--- 104,129 ----
  			? NULL \
  			: (struct lim_aux_data *) (stmt_ann (STMT)->aux))
  
! /* Description of a memory reference location for store motion.  */
  
! struct mem_ref_loc
  {
    tree *ref;			/* The reference itself.  */
    tree stmt;			/* The statement in that it occurs.  */
!   struct mem_ref_loc *next;	/* Next use in the chain.  */
! };
! 
! /* Description of a memory reference for store motion.  */
! 
! struct mem_ref
! {
!   tree mem;			/* The memory itself.  */
!   hashval_t hash;		/* Its hash value.  */
!   bool is_stored;		/* True if there is a store to the location
! 				   in the loop.  */
!   struct mem_ref_loc *locs;	/* The locations where it is found.  */
!   bitmap vops;			/* Vops corresponding to this memory
! 				   location.  */
  };
  
  /* Minimum cost of an expensive expression.  */
*************** struct mem_ref
*** 119,139 ****
     block will be executed.  */
  #define ALWAYS_EXECUTED_IN(BB) ((struct loop *) (BB)->aux)
  
- static unsigned max_stmt_uid;	/* Maximal uid of a statement.  Uids to phi
- 				   nodes are assigned using the versions of
- 				   ssa names they define.  */
- 
- /* Returns uid of statement STMT.  */
- 
- static unsigned
- get_stmt_uid (tree stmt)
- {
-   if (TREE_CODE (stmt) == PHI_NODE)
-     return SSA_NAME_VERSION (PHI_RESULT (stmt)) + max_stmt_uid;
- 
-   return stmt_ann (stmt)->uid;
- }
- 
  /* Calls CBCK for each index in memory reference ADDR_P.  There are two
     kinds situations handled; in each of these cases, the memory reference
     and DATA are passed to the callback:
--- 133,138 ----
*************** force_move_till (tree ref, tree *index, 
*** 859,871 ****
    return true;
  }
  
! /* Records memory reference *REF (that occurs in statement STMT)
!    to the list MEM_REFS.  */
  
  static void
! record_mem_ref (struct mem_ref **mem_refs, tree stmt, tree *ref)
  {
!   struct mem_ref *aref = xmalloc (sizeof (struct mem_ref));
  
    aref->stmt = stmt;
    aref->ref = ref;
--- 858,870 ----
    return true;
  }
  
! /* Records memory reference location *REF to the list MEM_REFS.  The reference
!    occurs in statement STMT.  */
  
  static void
! record_mem_ref_loc (struct mem_ref_loc **mem_refs, tree stmt, tree *ref)
  {
!   struct mem_ref_loc *aref = xmalloc (sizeof (struct mem_ref_loc));
  
    aref->stmt = stmt;
    aref->ref = ref;
*************** record_mem_ref (struct mem_ref **mem_ref
*** 874,885 ****
    *mem_refs = aref;
  }
  
! /* Releases list of memory references MEM_REFS.  */
  
  static void
! free_mem_refs (struct mem_ref *mem_refs)
  {
!   struct mem_ref *act;
  
    while (mem_refs)
      {
--- 873,884 ----
    *mem_refs = aref;
  }
  
! /* Releases list of memory reference locations MEM_REFS.  */
  
  static void
! free_mem_ref_locs (struct mem_ref_loc *mem_refs)
  {
!   struct mem_ref_loc *act;
  
    while (mem_refs)
      {
*************** free_mem_refs (struct mem_ref *mem_refs)
*** 889,1124 ****
      }
  }
  
- /* If VAR is defined in LOOP and the statement it is defined in does not belong
-    to the set SEEN, add the statement to QUEUE of length IN_QUEUE and
-    to the set SEEN.  */
- 
- static void
- maybe_queue_var (tree var, struct loop *loop,
- 		 sbitmap seen, tree *queue, unsigned *in_queue)
- {
-   tree stmt = SSA_NAME_DEF_STMT (var);
-   basic_block def_bb = bb_for_stmt (stmt);
- 	      
-   if (!def_bb
-       || !flow_bb_inside_loop_p (loop, def_bb)
-       || TEST_BIT (seen, get_stmt_uid (stmt)))
-     return;
- 	  
-   SET_BIT (seen, get_stmt_uid (stmt));
-   queue[(*in_queue)++] = stmt;
- }
- 
- /* If COMMON_REF is NULL, set COMMON_REF to *OP and return true.
-    Otherwise return true if the memory reference *OP is equal to COMMON_REF.
-    Record the reference OP to list MEM_REFS.  STMT is the statement in that
-    the reference occurs.  */
- 
- struct sra_data
- {
-   struct mem_ref **mem_refs;
-   tree common_ref;
-   tree stmt;
- };
- 
- static bool
- fem_single_reachable_address (tree *op, void *data)
- {
-   struct sra_data *sra_data = data;
- 
-   if (sra_data->common_ref
-       && !operand_equal_p (*op, sra_data->common_ref, 0))
-     return false;
-   sra_data->common_ref = *op;
- 
-   record_mem_ref (sra_data->mem_refs, sra_data->stmt, op);
-   return true;
- }
- 
- /* Runs CALLBACK for each operand of STMT that is a memory reference.  DATA
-    is passed to the CALLBACK as well.  The traversal stops if CALLBACK
-    returns false, for_each_memref then returns false as well.  Otherwise
-    for_each_memref returns true.  */
- 
- static bool
- for_each_memref (tree stmt, bool (*callback)(tree *, void *), void *data)
- {
-   tree *op;
- 
-   if (TREE_CODE (stmt) == RETURN_EXPR)
-     stmt = TREE_OPERAND (stmt, 1);
- 
-   if (TREE_CODE (stmt) == MODIFY_EXPR)
-     {
-       op = &TREE_OPERAND (stmt, 0);
-       if (TREE_CODE (*op) != SSA_NAME
- 	  && !callback (op, data))
- 	return false;
- 
-       op = &TREE_OPERAND (stmt, 1);
-       if (TREE_CODE (*op) != SSA_NAME
- 	  && is_gimple_lvalue (*op)
- 	  && !callback (op, data))
- 	return false;
- 
-       stmt = TREE_OPERAND (stmt, 1);
-     }
- 
-   if (TREE_CODE (stmt) == WITH_SIZE_EXPR)
-     stmt = TREE_OPERAND (stmt, 0);
- 
-   if (TREE_CODE (stmt) == CALL_EXPR)
-     {
-       tree args;
- 
-       for (args = TREE_OPERAND (stmt, 1); args; args = TREE_CHAIN (args))
- 	{
- 	  op = &TREE_VALUE (args);
- 
- 	  if (TREE_CODE (*op) != SSA_NAME
- 	      && is_gimple_lvalue (*op)
- 	      && !callback (op, data))
- 	    return false;
- 	}
-     }
- 
-   return true;
- }
- 
- /* Determine whether all memory references inside the LOOP that correspond
-    to virtual ssa names defined in statement STMT are equal.
-    If so, store the list of the references to MEM_REFS, and return one
-    of them.  Otherwise store NULL to MEM_REFS and return NULL_TREE.
-    *SEEN_CALL_STMT is set to true if the virtual operands suggest
-    that the reference might be clobbered by a call inside the LOOP.  */
- 
- static tree
- single_reachable_address (struct loop *loop, tree stmt,
- 			  struct mem_ref **mem_refs,
- 			  bool *seen_call_stmt)
- {
-   unsigned max_uid = max_stmt_uid + num_ssa_names;
-   tree *queue = xmalloc (sizeof (tree) * max_uid);
-   sbitmap seen = sbitmap_alloc (max_uid);
-   unsigned in_queue = 1;
-   unsigned i;
-   struct sra_data sra_data;
-   tree call;
-   tree val;
-   ssa_op_iter iter;
-   imm_use_iterator imm_iter;
-   use_operand_p use_p;
- 
-   sbitmap_zero (seen);
- 
-   *mem_refs = NULL;
-   sra_data.mem_refs = mem_refs;
-   sra_data.common_ref = NULL_TREE;
- 
-   queue[0] = stmt;
-   SET_BIT (seen, get_stmt_uid (stmt));
-   *seen_call_stmt = false;
- 
-   while (in_queue)
-     {
-       stmt = queue[--in_queue];
-       sra_data.stmt = stmt;
- 
-       if (LIM_DATA (stmt)
- 	  && LIM_DATA (stmt)->sm_done)
- 	goto fail;
- 
-       switch (TREE_CODE (stmt))
- 	{
- 	case MODIFY_EXPR:
- 	case CALL_EXPR:
- 	case RETURN_EXPR:
- 	  if (!for_each_memref (stmt, fem_single_reachable_address,
- 				&sra_data))
- 	    goto fail;
- 
- 	  /* If this is a function that may depend on the memory location,
- 	     record the fact.  We cannot directly refuse call clobbered
- 	     operands here, since sra_data.common_ref did not have
- 	     to be set yet.  */
- 	  call = get_call_expr_in (stmt);
- 	  if (call
- 	      && !(call_expr_flags (call) & ECF_CONST))
- 	    *seen_call_stmt = true;
- 
- 	  /* Traverse also definitions of the VUSES (there may be other
- 	     distinct from the one we used to get to this statement).  */
- 	  FOR_EACH_SSA_TREE_OPERAND (val, stmt, iter, SSA_OP_VIRTUAL_USES)
- 	    maybe_queue_var (val, loop, seen, queue, &in_queue);
- 
- 	  break;
- 
- 	case PHI_NODE:
- 	  for (i = 0; i < (unsigned) PHI_NUM_ARGS (stmt); i++)
- 	    if (TREE_CODE (PHI_ARG_DEF (stmt, i)) == SSA_NAME)
- 	      maybe_queue_var (PHI_ARG_DEF (stmt, i), loop,
- 		               seen, queue, &in_queue);
- 	  break;
- 
- 	default:
- 	  goto fail;
- 	}
- 
-       /* Find uses of virtual names.  */
-       if (TREE_CODE (stmt) == PHI_NODE)
-         {
- 	  if (!is_gimple_reg (SSA_NAME_VAR (PHI_RESULT (stmt))))
- 	    FOR_EACH_IMM_USE_FAST (use_p, imm_iter, PHI_RESULT (stmt))
- 	      {	      
- 		tree imm_stmt = USE_STMT (use_p);
- 
- 		if (TEST_BIT (seen, get_stmt_uid (imm_stmt)))
- 		  continue;
- 
- 		if (!flow_bb_inside_loop_p (loop, bb_for_stmt (imm_stmt)))
- 		  continue;
- 
- 		SET_BIT (seen, get_stmt_uid (imm_stmt));
- 
- 		queue[in_queue++] = imm_stmt;
- 	      }
- 	}
-       else
- 	FOR_EACH_SSA_TREE_OPERAND (val, stmt, iter, SSA_OP_VIRTUAL_DEFS)
- 	  FOR_EACH_IMM_USE_FAST (use_p, imm_iter, val)
- 	    {
- 	      tree imm_stmt = USE_STMT (use_p);
- 
- 	      if (TEST_BIT (seen, get_stmt_uid (imm_stmt)))
- 		continue;
- 
- 	      if (!flow_bb_inside_loop_p (loop, bb_for_stmt (imm_stmt)))
- 		continue;
- 
- 	      SET_BIT (seen, get_stmt_uid (imm_stmt));
- 
- 	      queue[in_queue++] = imm_stmt;
- 	    }
-     }
- 
-   free (queue);
-   sbitmap_free (seen);
- 
-   return sra_data.common_ref;
- 
- fail:
-   free_mem_refs (*mem_refs);
-   *mem_refs = NULL;
-   free (queue);
-   sbitmap_free (seen);
- 
-   return NULL;
- }
- 
  /* Rewrites memory references in list MEM_REFS by variable TMP_VAR.  */
  
  static void
! rewrite_mem_refs (tree tmp_var, struct mem_ref *mem_refs)
  {
    tree var;
    ssa_op_iter iter;
--- 888,897 ----
      }
  }
  
  /* Rewrites memory references in list MEM_REFS by variable TMP_VAR.  */
  
  static void
! rewrite_mem_refs (tree tmp_var, struct mem_ref_loc *mem_refs)
  {
    tree var;
    ssa_op_iter iter;
*************** rewrite_mem_refs (tree tmp_var, struct m
*** 1143,1151 ****
  
  static void
  schedule_sm (struct loop *loop, edge *exits, unsigned n_exits, tree ref,
! 	     struct mem_ref *mem_refs)
  {
!   struct mem_ref *aref;
    tree tmp_var;
    unsigned i;
    tree load, store;
--- 916,924 ----
  
  static void
  schedule_sm (struct loop *loop, edge *exits, unsigned n_exits, tree ref,
! 	     struct mem_ref_loc *mem_refs)
  {
!   struct mem_ref_loc *aref;
    tree tmp_var;
    unsigned i;
    tree load, store;
*************** schedule_sm (struct loop *loop, edge *ex
*** 1187,1290 ****
      }
  }
  
! /* Returns true if REF may be clobbered by calls.  */
! 
! static bool
! is_call_clobbered_ref (tree ref)
! {
!   tree base;
!   HOST_WIDE_INT offset, size;
!   subvar_t sv;
!   subvar_t svars;
!   tree sref = ref;
! 
!   if (TREE_CODE (sref) == COMPONENT_REF
!       && (sref = okay_component_ref_for_subvars (sref, &offset, &size)))
!     {
!       svars = get_subvars_for_var (sref);
!       for (sv = svars; sv; sv = sv->next)
! 	{
! 	  if (overlap_subvar (offset, size, sv, NULL)
! 	      && is_call_clobbered (sv->var))
! 	    return true;
! 	}
!     }
! 	      
!   base = get_base_address (ref);
!   if (!base)
!     return true;
! 
!   if (DECL_P (base))
!     {
!       if (var_can_have_subvars (base)
! 	  && (svars = get_subvars_for_var (base)))
! 	{
! 	  for (sv = svars; sv; sv = sv->next)
! 	    if (is_call_clobbered (sv->var))
! 	      return true;
! 	  return false;
! 	}
!       else
! 	return is_call_clobbered (base);
!     }
! 
!   if (INDIRECT_REF_P (base))
!     {
!       /* Check whether the alias tags associated with the pointer
! 	 are call clobbered.  */
!       tree ptr = TREE_OPERAND (base, 0);
!       struct ptr_info_def *pi = SSA_NAME_PTR_INFO (ptr);
!       tree nmt = (pi) ? pi->name_mem_tag : NULL_TREE;
!       tree tmt = var_ann (SSA_NAME_VAR (ptr))->type_mem_tag;
! 
!       if ((nmt && is_call_clobbered (nmt))
! 	  || (tmt && is_call_clobbered (tmt)))
! 	return true;
! 
!       return false;
!     }
! 
!   gcc_unreachable ();
! }
! 
! /* Determine whether all memory references inside LOOP corresponding to the
!    virtual ssa name REG are equal to each other, and whether the address of
!    this common reference can be hoisted outside of the loop.  If this is true,
!    prepare the statements that load the value of the memory reference to a
!    temporary variable in the loop preheader, store it back on the loop exits,
!    and replace all the references inside LOOP by this temporary variable.
!    LOOP has N_EXITS stored in EXITS.  */
  
  static void
! determine_lsm_reg (struct loop *loop, edge *exits, unsigned n_exits, tree reg)
  {
!   tree ref;
!   struct mem_ref *mem_refs, *aref;
    struct loop *must_exec;
-   bool sees_call;
-   
-   if (is_gimple_reg (reg))
-     return;
-   
-   ref = single_reachable_address (loop, SSA_NAME_DEF_STMT (reg), &mem_refs,
- 				  &sees_call);
-   if (!ref)
-     return;
  
!   /* If we cannot create a ssa name for the result, give up.  */
!   if (!is_gimple_reg_type (TREE_TYPE (ref))
!       || TREE_THIS_VOLATILE (ref))
!     goto fail;
! 
!   /* If there is a call that may use the location, give up as well.  */
!   if (sees_call
!       && is_call_clobbered_ref (ref))
!     goto fail;
  
!   if (!for_each_index (&ref, may_move_till, loop))
!     goto fail;
  
!   if (tree_could_trap_p (ref))
      {
        /* If the memory access is unsafe (i.e. it might trap), ensure that some
  	 of the statements in that it occurs is always executed when the loop
--- 960,990 ----
      }
  }
  
! /* Check whether memory reference REF can be hoisted out of the LOOP.  If this
!    is true, prepare the statements that load the value of the memory reference
!    to a temporary variable in the loop preheader, store it back on the loop
!    exits, and replace all the references inside LOOP by this temporary variable.
!    LOOP has N_EXITS stored in EXITS.  CLOBBERED_VOPS is the bitmap of virtual
!    operands that are clobbered by a call or accessed through multiple references
!    in loop.  */
  
  static void
! determine_lsm_ref (struct loop *loop, edge *exits, unsigned n_exits,
! 		   bitmap clobbered_vops, struct mem_ref *ref)
  {
!   struct mem_ref_loc *aref;
    struct loop *must_exec;
  
!   /* In case the memory is not stored to, there is nothing for SM to do.  */
!   if (!ref->is_stored)
!     return;
  
!   /* If the reference is aliased with any different ref, or killed by call
!      in function, then fail.  */
!   if (bitmap_intersect_p (ref->vops, clobbered_vops))
!     return;
  
!   if (tree_could_trap_p (ref->mem))
      {
        /* If the memory access is unsafe (i.e. it might trap), ensure that some
  	 of the statements in that it occurs is always executed when the loop
*************** determine_lsm_reg (struct loop *loop, ed
*** 1297,1303 ****
  	 least one of the statements containing the memory reference is
  	 executed.  */
  
!       for (aref = mem_refs; aref; aref = aref->next)
  	{
  	  if (!LIM_DATA (aref->stmt))
  	    continue;
--- 997,1003 ----
  	 least one of the statements containing the memory reference is
  	 executed.  */
  
!       for (aref = ref->locs; aref; aref = aref->next)
  	{
  	  if (!LIM_DATA (aref->stmt))
  	    continue;
*************** determine_lsm_reg (struct loop *loop, ed
*** 1312,1324 ****
  	}
  
        if (!aref)
! 	goto fail;
      }
  
!   schedule_sm (loop, exits, n_exits, ref, mem_refs);
  
! fail: ;
!   free_mem_refs (mem_refs);
  }
  
  /* Checks whether LOOP (with N_EXITS exits stored in EXITS array) is suitable
--- 1012,1045 ----
  	}
  
        if (!aref)
! 	return;
      }
  
!   schedule_sm (loop, exits, n_exits, ref->mem, ref->locs);
! }
! 
! /* Attempts to hoist memory reference described in SLOT out of loop
!    described in DATA.  Callback for htab_traverse.  */
! 
! struct hmr_data
! {
!   struct loop *loop;	/* Loop from that the reference should be hoisted.  */
!   edge *exits;		/* Exits of the loop.  */
!   unsigned n_exits;	/* Length of the exits array.  */
!   bitmap clobbered_vops;/* The vops clobbered by call in loop or accessed by
! 			   multiple memory references.  */
! };
! 
! static int
! hoist_memory_reference (void **slot, void *data)
! {
!   struct mem_ref *ref = *slot;
!   struct hmr_data *hmr_data = data;
! 
!   determine_lsm_ref (hmr_data->loop, hmr_data->exits, hmr_data->n_exits,
! 		     hmr_data->clobbered_vops, ref);
  
!   return 1;
  }
  
  /* Checks whether LOOP (with N_EXITS exits stored in EXITS array) is suitable
*************** loop_suitable_for_sm (struct loop *loop 
*** 1338,1352 ****
    return true;
  }
  
  /* Try to perform store motion for all memory references modified inside
     LOOP.  */
  
  static void
  determine_lsm_loop (struct loop *loop)
  {
-   tree phi;
    unsigned n_exits;
    edge *exits = get_loop_exit_edges (loop, &n_exits);
  
    if (!loop_suitable_for_sm (loop, exits, n_exits))
      {
--- 1059,1245 ----
    return true;
  }
  
+ /* A hash function for struct mem_ref object OBJ.  */
+ 
+ static hashval_t
+ memref_hash (const void *obj)
+ {
+   const struct mem_ref *mem = obj;
+ 
+   return mem->hash;
+ }
+ 
+ /* An equality function for struct mem_ref object OBJ1 with
+    memory reference OBJ2.  */
+ 
+ static int
+ memref_eq (const void *obj1, const void *obj2)
+ {
+   const struct mem_ref *mem1 = obj1;
+ 
+   return operand_equal_p (mem1->mem, (tree) obj2, 0);
+ }
+ 
+ /* A function to free the struct mem_ref object OBJ.  */
+ 
+ static void
+ memref_del (void *obj)
+ {
+   struct mem_ref *mem = obj;
+ 
+   free_mem_ref_locs (mem->locs);
+   BITMAP_FREE (mem->vops);
+   free (mem);
+ }
+ 
+ /* Gathers memory references in statement STMT in LOOP, storing the
+    information about them in MEM_REFS hash table.  Note vops accessed through
+    unrecognized statements in CLOBBERED_VOPS.  */
+ 
+ static void
+ gather_mem_refs_stmt (struct loop *loop, htab_t mem_refs,
+ 		      bitmap clobbered_vops, tree stmt)
+ {
+   tree *lhs, *rhs, *mem = NULL;
+   hashval_t hash;
+   PTR *slot;
+   struct mem_ref *ref = NULL;
+   ssa_op_iter oi;
+   tree vname;
+   bool is_stored;
+ 
+   if (ZERO_SSA_OPERANDS (stmt, SSA_OP_ALL_VIRTUALS))
+     return;
+ 
+   /* Recognize MEM = (SSA_NAME | invariant) and SSA_NAME = MEM patterns.  */
+   if (TREE_CODE (stmt) != MODIFY_EXPR)
+     goto fail;
+ 
+   lhs = &TREE_OPERAND (stmt, 0);
+   rhs = &TREE_OPERAND (stmt, 1);
+ 
+   if (TREE_CODE (*lhs) == SSA_NAME)
+     {
+       if (!is_gimple_addressable (*rhs))
+ 	goto fail;
+ 
+       mem = rhs;
+       is_stored = false;
+     }
+   else if (TREE_CODE (*rhs) == SSA_NAME
+ 	   || is_gimple_min_invariant (*rhs))
+     {
+       mem = lhs;
+       is_stored = true;
+     }
+   else
+     goto fail;
+ 
+   /* If we cannot create a ssa name for the result, give up.  */
+   if (!is_gimple_reg_type (TREE_TYPE (*mem))
+       || TREE_THIS_VOLATILE (*mem))
+     goto fail;
+ 
+   /* If we cannot move the reference from the loop, fail.  */
+   if (!for_each_index (mem, may_move_till, loop))
+     goto fail;
+ 
+   hash = iterative_hash_expr (*mem, 0);
+   slot = htab_find_slot_with_hash (mem_refs, *mem, hash, INSERT);
+ 
+   if (*slot)
+     ref = *slot;
+   else
+     {
+       ref = xmalloc (sizeof (struct mem_ref));
+       ref->mem = *mem;
+       ref->hash = hash;
+       ref->locs = NULL;
+       ref->is_stored = false;
+       ref->vops = BITMAP_ALLOC (NULL);
+       *slot = ref;
+     }
+   ref->is_stored |= is_stored;
+ 
+   FOR_EACH_SSA_TREE_OPERAND (vname, stmt, oi,
+ 			     SSA_OP_VIRTUAL_USES | SSA_OP_VIRTUAL_KILLS)
+     {
+       bitmap_set_bit (ref->vops,
+ 		      var_ann (SSA_NAME_VAR (vname))->uid);
+     }
+   record_mem_ref_loc (&ref->locs, stmt, mem);
+   return;
+ 
+ fail:
+   FOR_EACH_SSA_TREE_OPERAND (vname, stmt, oi,
+ 			     SSA_OP_VIRTUAL_USES | SSA_OP_VIRTUAL_KILLS)
+     {
+       bitmap_set_bit (clobbered_vops,
+ 		      var_ann (SSA_NAME_VAR (vname))->uid);
+     }
+ }
+ 
+ /* Gathers memory references in LOOP, storing the information about them
+    in MEM_REFS hash table.  Note vops accessed through unrecognized
+    statements in CLOBBERED_VOPS.  */
+ 
+ static void
+ gather_mem_refs (struct loop *loop, htab_t mem_refs, bitmap clobbered_vops)
+ {
+   basic_block *body = get_loop_body (loop);
+   block_stmt_iterator bsi;
+   unsigned i;
+ 
+   for (i = 0; i < loop->num_nodes; i++)
+     {
+       for (bsi = bsi_start (body[i]); !bsi_end_p (bsi); bsi_next (&bsi))
+ 	gather_mem_refs_stmt (loop, mem_refs, clobbered_vops, bsi_stmt (bsi));
+     }
+ 
+   free (body);
+ }
+ 
+ /* Finds the vops accessed by memory reference described in SLOT as well as
+    some other reference(s) and marks them in DATA->clobbered_vops.
+    Callback for htab_traverse.  */
+ 
+ struct fmrv_data
+ {
+   bitmap clobbered_vops;	/* The vops clobbered by call in loop or accessed by
+ 			   multiple memory references.  */
+   bitmap all_vops;	/* All vops referenced in the loop.  */
+ };
+ 
+ static int
+ find_more_ref_vops (void **slot, void *data)
+ {
+   struct mem_ref *ref = *slot;
+   struct fmrv_data *fmrv_data = data;
+   bitmap_head tmp;
+ 
+   /* The vops that are already in all_vops are accessed by more than
+      one memory reference.  */
+   bitmap_initialize (&tmp, &bitmap_default_obstack);
+   bitmap_and (&tmp, fmrv_data->all_vops, ref->vops);
+   bitmap_ior_into (fmrv_data->clobbered_vops, &tmp);
+   bitmap_clear (&tmp);
+ 
+   bitmap_ior_into (fmrv_data->all_vops, ref->vops);
+   return 1;
+ }
+ 
  /* Try to perform store motion for all memory references modified inside
     LOOP.  */
  
  static void
  determine_lsm_loop (struct loop *loop)
  {
    unsigned n_exits;
    edge *exits = get_loop_exit_edges (loop, &n_exits);
+   htab_t mem_refs;
+   struct hmr_data hmr_data;
+   struct fmrv_data fmrv_data;
+   bitmap clobbered_vops;
  
    if (!loop_suitable_for_sm (loop, exits, n_exits))
      {
*************** determine_lsm_loop (struct loop *loop)
*** 1354,1363 ****
        return;
      }
  
!   for (phi = phi_nodes (loop->header); phi; phi = PHI_CHAIN (phi))
!     determine_lsm_reg (loop, exits, n_exits, PHI_RESULT (phi));
  
    free (exits);
  }
  
  /* Try to perform store motion for all memory references modified inside
--- 1247,1274 ----
        return;
      }
  
!   mem_refs = htab_create (100, memref_hash, memref_eq, memref_del);
! 
!   /* Find the memory references in LOOP.  */
!   clobbered_vops = BITMAP_ALLOC (NULL);
!   gather_mem_refs (loop, mem_refs, clobbered_vops);
! 
!   /* Find the vops that are used for more than one reference.  */
!   fmrv_data.all_vops = BITMAP_ALLOC (NULL);
!   fmrv_data.clobbered_vops = clobbered_vops;
!   htab_traverse (mem_refs, find_more_ref_vops, &fmrv_data);
!   BITMAP_FREE (fmrv_data.all_vops);
! 
!   /* Hoist all suitable memory references.  */
!   hmr_data.loop = loop;
!   hmr_data.exits = exits;
!   hmr_data.n_exits = n_exits;
!   hmr_data.clobbered_vops = clobbered_vops;
!   htab_traverse (mem_refs, hoist_memory_reference, &hmr_data);
  
+   htab_delete (mem_refs);
    free (exits);
+   BITMAP_FREE (clobbered_vops);
  }
  
  /* Try to perform store motion for all memory references modified inside
*************** static void
*** 1367,1391 ****
  determine_lsm (struct loops *loops)
  {
    struct loop *loop;
-   basic_block bb;
  
    if (!loops->tree_root->inner)
      return;
  
!   /* Create a UID for each statement in the function.  Ordering of the
!      UIDs is not important for this pass.  */
!   max_stmt_uid = 0;
!   FOR_EACH_BB (bb)
!     {
!       block_stmt_iterator bsi;
! 
!       for (bsi = bsi_start (bb); !bsi_end_p (bsi); bsi_next (&bsi))
! 	stmt_ann (bsi_stmt (bsi))->uid = max_stmt_uid++;
!     }
! 
!   /* Pass the loops from the outermost.  For each virtual operand loop phi node
!      check whether all the references inside the loop correspond to a single
!      address, and if so, move them.  */
  
    loop = loops->tree_root->inner;
    while (1)
--- 1278,1289 ----
  determine_lsm (struct loops *loops)
  {
    struct loop *loop;
  
    if (!loops->tree_root->inner)
      return;
  
!   /* Pass the loops from the outermost and perform the store motion as
!      suitable.  */
  
    loop = loops->tree_root->inner;
    while (1)

Re: Patches ping

[Diego Novillo]

On Fri, May 06, 2005 at 11:09:33PM +0200, Zdenek Dvorak wrote:

> 	* tree-ssa-loop-im.c: Include hashtab.h.
> 	(struct mem_ref_loc): New.
> 	(struct mem_ref): Describe the set of references with the same
> 	shape.
> 	(max_stmt_uid, get_stmt_uid, record_mem_ref, free_mem_refs,
> 	maybe_queue_var, fem_single_reachable_address,
> 	for_each_memref, single_reachable_address,
> 	is_call_clobbered_ref, determine_lsm_reg): Removed.
> 	(record_mem_ref_loc, free_mem_ref_locs, determine_lsm_ref,
> 	hoist_memory_reference, memref_hash, memref_eq, memref_del,
> 	gather_mem_refs_stmt, gather_mem_refs, find_more_ref_vops):
> 	New functions.
> 	(rewrite_mem_refs, schedule_sm): Use mem_ref_loc list.
> 	(determine_lsm_loop): Rewritten.
> 	(determine_lsm): Do not set stmt uids.
> 
This version is much easier to follow than the original.  Thanks.
The patch is fine.  A couple of minor comment fixes:

> + 
> +   /* If we cannot create a ssa name for the result, give up.  */
                             ^^^^^
			     an SSA

> +   if (!is_gimple_reg_type (TREE_TYPE (*mem))
> +       || TREE_THIS_VOLATILE (*mem))
> +     goto fail;
> + 
> +   /* If we cannot move the reference from the loop, fail.  */
                                         ^^^^
					 out of


Diego.

Re: Patches ping

[Geoffrey Keating]

Zdenek Dvorak <rakdver@atrey.karlin.mff.cuni.cz> writes:

> http://gcc.gnu.org/ml/gcc-patches/2005-01/msg00796.html
>   -- better heuristics for complete loop unrolling (PR 19401)

This is OK.

> http://gcc.gnu.org/ml/gcc-patches/2005-01/msg01644.html
>   -- the patch to include ggc memory consumption report
>      to the standard timevar report

This is OK.

Patches ping

[Revital Eres]

Hello,

[PATCH, SMS] Prevent the creation of reg-moves for definitions with MODE_CC
http://gcc.gnu.org/ml/gcc-patches/2011-12/msg01459.html

[PATCH SMS 2/2, RFC] Register pressure estimation for the partial schedule
http://gcc.gnu.org/ml/gcc-patches/2011-12/msg01330.html

Thanks,
Revital

Patches ping

[Revital Eres]

Hello,

[PATCH, SMS 3/4] Optimize stage count
http://gcc.gnu.org/ml/gcc-patches/2011-05/msg01341.html

[PATCH, SMS 4/4] Misc. fixes
http://gcc.gnu.org/ml/gcc-patches/2011-05/msg01342.html

[PATCH, SMS] Fix calculation of issue_rate
http://gcc.gnu.org/ml/gcc-patches/2011-05/msg01344.html

Thanks,
Revital

Re: Patches ping

[Ayal Zaks]

> [PATCH, SMS 3/4] Optimize stage count
> http://gcc.gnu.org/ml/gcc-patches/2011-05/msg01341.html

This patch for minimizing the stage count (which also refactors and
cleans up the code) is approved. Have some minor comments below,
followed by some thoughts for possible follow-up improvements.
Thanks,
Ayal.

>Changelog:
>
>       (sms_schedule_by_order): Update call to get_sched_window.
>       all set_must_precede_follow.
        ^^^
        call


>+/* Set bitmaps TMP_FOLLOW and TMP_PRECEDE to MUST_FOLLOW and MUST_PRECEDE
>+   respectively only if cycle C falls in the scheduling window boundaries
                                        ^^
                                 on the border of


>                   sbitmap tmp_precede = NULL;
>                   sbitmap tmp_follow = NULL;
are redundantly reset in set_must_precede_follow().



>+/* Update the sched_params for node U using the II,
                            ^
                            (time, row and stage)
 >+   the CYCLE of U and MIN_CYCLE.  */
Please also clarify that we're not simply taking
  SCHED_STAGE (u) = CALC_STAGE_COUNT (SCHED_TIME (u), min_cycle, ii);
because the stages are aligned on cycle 0.


>+  /* First, normailize the partial schedualing.  */
                   ^                       ^

>+   /* Try to achieve optimized SC by normalizing the partial
>+      schedule (having the cycles start from cycle zero). The branch
                                                           ^
>+      location must be placed in row ii-1 in the final scheduling.

>+      If that's not the case after the normalization then try to
        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>+      move the branch to that row if possible.  */
        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
        If failed, shift all instructions to position the branch in row ii-1.


For consistency and clarity, may be instead of:

>+       /* Bring the branch to cycle -1.  */
>+       int amount = SCHED_TIME (g->closing_branch) + 1;
it would be better to have:

+       /* Bring the branch to cycle ii-1.  */
+       int amount = SCHED_TIME (g->closing_branch) - (ii - 1);


Some thoughts on possible improvements (not mandatory; especially
given the delay in approval, sorry..thanks for the ping):
o Have optimize_sc() take care of all possible rotations doing the
best it can, without returning an indication which leaves subsequent
(suboptimal) processing to the caller.
o Instead of removing the branch and then trying to get it back into
the same cycle if you can't place it in row ii-1, consider keeping it
in its place and removing it only if you succeed to schedule it
(also..) in row ii-1.
o May be worthwhile to apply more refactoring, so that the code to
reschedule the branch in row ii-1 reuses more of the general code for
scheduling an instruction (possibly setting end = start + end), but
avoid splitting a row.
o Would be interesting to learn of loops that still have suboptimal
SC's, to see if it's still an issue.
Ayal.


2011/7/20 Revital Eres <revital.eres@linaro.org>
>
> Hello,
>
> [PATCH, SMS 3/4] Optimize stage count
> http://gcc.gnu.org/ml/gcc-patches/2011-05/msg01341.html
>
> [PATCH, SMS 4/4] Misc. fixes
> http://gcc.gnu.org/ml/gcc-patches/2011-05/msg01342.html
>
> [PATCH, SMS] Fix calculation of issue_rate
> http://gcc.gnu.org/ml/gcc-patches/2011-05/msg01344.html
>
> Thanks,
> Revital

Re: Patches ping

[Revital1 Eres]

[-- Attachment #1: Type: text/plain, Size: 3521 bytes --]


Hi,

Thanks for the review!

>
> >Changelog:
> >
> >       (sms_schedule_by_order): Update call to get_sched_window.
> >       all set_must_precede_follow.
>         ^^^
>         call

Done.

> >+/* Set bitmaps TMP_FOLLOW and TMP_PRECEDE to MUST_FOLLOW and
MUST_PRECEDE
> >+   respectively only if cycle C falls in the scheduling window
boundaries
>                                         ^^
>                                  on the border of

Done.

>
>
> >                   sbitmap tmp_precede = NULL;
> >                   sbitmap tmp_follow = NULL;
> are redundantly reset in set_must_precede_follow().

Done. I removed the setting of tmp_precede and tmp_follow before calling
set_must_precede_follow. (they are reset in set_must_precede_follow)

> >+/* Update the sched_params for node U using the II,
>                             ^
>                             (time, row and stage)
>  >+   the CYCLE of U and MIN_CYCLE.  */
> Please also clarify that we're not simply taking
>   SCHED_STAGE (u) = CALC_STAGE_COUNT (SCHED_TIME (u), min_cycle, ii);
> because the stages are aligned on cycle 0.

Done. I assume it should be "because the stages may not be aligned on cycle
0."

> >+  /* First, normailize the partial schedualing.  */
>                    ^                       ^
>
> >+   /* Try to achieve optimized SC by normalizing the partial
> >+      schedule (having the cycles start from cycle zero). The branch
>                                                            ^
> >+      location must be placed in row ii-1 in the final scheduling.
>
> >+      If that's not the case after the normalization then try to
>         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> >+      move the branch to that row if possible.  */
>         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>         If failed, shift all instructions to position the branch in row
ii-1.

Done.


> For consistency and clarity, may be instead of:
>
> >+       /* Bring the branch to cycle -1.  */
> >+       int amount = SCHED_TIME (g->closing_branch) + 1;
> it would be better to have:
>
> +       /* Bring the branch to cycle ii-1.  */
> +       int amount = SCHED_TIME (g->closing_branch) - (ii - 1);
>

OK, the attached patch contains this fix in other places
as well where we bring the branch to cycle -1.

> Some thoughts on possible improvements (not mandatory; especially
> given the delay in approval, sorry..thanks for the ping):
> o Have optimize_sc() take care of all possible rotations doing the
> best it can, without returning an indication which leaves subsequent
> (suboptimal) processing to the caller.
> o Instead of removing the branch and then trying to get it back into
> the same cycle if you can't place it in row ii-1, consider keeping it
> in its place and removing it only if you succeed to schedule it
> (also..) in row ii-1.
> o May be worthwhile to apply more refactoring, so that the code to
> reschedule the branch in row ii-1 reuses more of the general code for
> scheduling an instruction (possibly setting end = start + end), but
> avoid splitting a row.
> o Would be interesting to learn of loops that still have suboptimal
> SC's, to see if it's still an issue.

Thanks for the suggestions, will try to address them.

I'm now re-testing the attached patch on PowerPC and will commit it if
there will be no further comments.

Thanks,
Revital

(See attached file: patch_opt_sc_1_8.txt)

[-- Attachment #2: patch_opt_sc_1_8.txt --]
[-- Type: text/plain, Size: 16025 bytes --]

Index: ChangeLog
===================================================================
--- ChangeLog	(revision 176998)
+++ ChangeLog	(working copy)
@@ -1,3 +1,18 @@
+2011-08-01  Revital Eres  <revital.eres@linaro.org>
+
+	* modulo-sched.c (calculate_stage_count,
+	calculate_must_precede_follow, get_sched_window,
+	try_scheduling_node_in_cycle, remove_node_from_ps): Add
+	declaration.
+	(update_node_sched_params, set_must_precede_follow, optimize_sc):
+	New functions.
+	(reset_sched_times): Call update_node_sched_params.
+	(sms_schedule): Call optimize_sc.
+	(get_sched_window): Change function arguments.
+	(sms_schedule_by_order): Update call to get_sched_window.
+	Call set_must_precede_follow.
+	(calculate_stage_count): Add function argument.
+
 2011-07-31  Richard Henderson  <rth@redhat.com>
 
 	* config/h8300/crti.asm: Add flags to .section directive.
Index: modulo-sched.c
===================================================================
--- modulo-sched.c	(revision 176998)
+++ modulo-sched.c	(working copy)
@@ -203,7 +203,16 @@ static void generate_prolog_epilog (part
                                     rtx, rtx);
 static void duplicate_insns_of_cycles (partial_schedule_ptr,
 				       int, int, int, rtx);
-static int calculate_stage_count (partial_schedule_ptr ps);
+static int calculate_stage_count (partial_schedule_ptr, int);
+static void calculate_must_precede_follow (ddg_node_ptr, int, int,
+					   int, int, sbitmap, sbitmap, sbitmap);
+static int get_sched_window (partial_schedule_ptr, ddg_node_ptr, 
+			     sbitmap, int, int *, int *, int *);
+static bool try_scheduling_node_in_cycle (partial_schedule_ptr, ddg_node_ptr,
+					  int, int, sbitmap, int *, sbitmap,
+					  sbitmap);
+static bool remove_node_from_ps (partial_schedule_ptr, ps_insn_ptr);
+
 #define SCHED_ASAP(x) (((node_sched_params_ptr)(x)->aux.info)->asap)
 #define SCHED_TIME(x) (((node_sched_params_ptr)(x)->aux.info)->time)
 #define SCHED_FIRST_REG_MOVE(x) \
@@ -576,6 +585,36 @@ free_undo_replace_buff (struct undo_repl
     }
 }
 
+/* Update the sched_params (time, row and stage) for node U using the II,
+   the CYCLE of U and MIN_CYCLE.  
+   We're not simply taking the following
+   SCHED_STAGE (u) = CALC_STAGE_COUNT (SCHED_TIME (u), min_cycle, ii);
+   because the stages may not be aligned on cycle 0.  */
+static void
+update_node_sched_params (ddg_node_ptr u, int ii, int cycle, int min_cycle)
+{
+  int sc_until_cycle_zero;
+  int stage;
+
+  SCHED_TIME (u) = cycle;
+  SCHED_ROW (u) = SMODULO (cycle, ii);
+
+  /* The calculation of stage count is done adding the number
+     of stages before cycle zero and after cycle zero.  */
+  sc_until_cycle_zero = CALC_STAGE_COUNT (-1, min_cycle, ii);
+
+  if (SCHED_TIME (u) < 0)
+    {
+      stage = CALC_STAGE_COUNT (-1, SCHED_TIME (u), ii);
+      SCHED_STAGE (u) = sc_until_cycle_zero - stage;
+    }
+  else
+    {
+      stage = CALC_STAGE_COUNT (SCHED_TIME (u), 0, ii);
+      SCHED_STAGE (u) = sc_until_cycle_zero + stage - 1;
+    }
+}
+
 /* Bump the SCHED_TIMEs of all nodes by AMOUNT.  Set the values of
    SCHED_ROW and SCHED_STAGE.  Instruction scheduled on cycle AMOUNT
    will move to cycle zero.  */
@@ -592,7 +631,6 @@ reset_sched_times (partial_schedule_ptr 
 	ddg_node_ptr u = crr_insn->node;
 	int normalized_time = SCHED_TIME (u) - amount;
 	int new_min_cycle = PS_MIN_CYCLE (ps) - amount;
-        int sc_until_cycle_zero, stage;
 
         if (dump_file)
           {
@@ -608,23 +646,9 @@ reset_sched_times (partial_schedule_ptr 
 	
 	gcc_assert (SCHED_TIME (u) >= ps->min_cycle);
 	gcc_assert (SCHED_TIME (u) <= ps->max_cycle);
-	SCHED_TIME (u) = normalized_time;
-	SCHED_ROW (u) = SMODULO (normalized_time, ii);
-      
-        /* The calculation of stage count is done adding the number
-           of stages before cycle zero and after cycle zero.  */
-	sc_until_cycle_zero = CALC_STAGE_COUNT (-1, new_min_cycle, ii);
-	
-	if (SCHED_TIME (u) < 0)
-	  {
-	    stage = CALC_STAGE_COUNT (-1, SCHED_TIME (u), ii);
-	    SCHED_STAGE (u) = sc_until_cycle_zero - stage;
-	  }
-	else
-	  {
-	    stage = CALC_STAGE_COUNT (SCHED_TIME (u), 0, ii);
-	    SCHED_STAGE (u) = sc_until_cycle_zero + stage - 1;
-	  }
+
+	crr_insn->cycle = normalized_time;
+	update_node_sched_params (u, ii, normalized_time, new_min_cycle);
       }
 }
  
@@ -661,6 +685,206 @@ permute_partial_schedule (partial_schedu
 			    PREV_INSN (last));
 }
 
+/* Set bitmaps TMP_FOLLOW and TMP_PRECEDE to MUST_FOLLOW and MUST_PRECEDE
+   respectively only if cycle C falls on the border of the scheduling
+   window boundaries marked by START and END cycles.  STEP is the
+   direction of the window.  */
+static inline void
+set_must_precede_follow (sbitmap *tmp_follow, sbitmap must_follow,
+			 sbitmap *tmp_precede, sbitmap must_precede, int c,
+			 int start, int end, int step)
+{
+  *tmp_precede = NULL;
+  *tmp_follow = NULL;
+
+  if (c == start)
+    {
+      if (step == 1)
+	*tmp_precede = must_precede;
+      else			/* step == -1.  */
+	*tmp_follow = must_follow;
+    }
+  if (c == end - step)
+    {
+      if (step == 1)
+	*tmp_follow = must_follow;
+      else			/* step == -1.  */
+	*tmp_precede = must_precede;
+    }
+
+}
+
+/* Return True if the branch can be moved to row ii-1 while
+   normalizing the partial schedule PS to start from cycle zero and thus
+   optimize the SC.  Otherwise return False.  */
+static bool
+optimize_sc (partial_schedule_ptr ps, ddg_ptr g)
+{
+  int amount = PS_MIN_CYCLE (ps);
+  sbitmap sched_nodes = sbitmap_alloc (g->num_nodes);
+  int start, end, step;
+  int ii = ps->ii;
+  bool ok = false;
+  int stage_count, stage_count_curr;
+
+  /* Compare the SC after normalization and SC after bringing the branch
+     to row ii-1.  If they are equal just bail out.  */
+  stage_count = calculate_stage_count (ps, amount);
+  stage_count_curr =
+    calculate_stage_count (ps, SCHED_TIME (g->closing_branch) - (ii - 1));
+
+  if (stage_count == stage_count_curr)
+    {
+      if (dump_file)
+	fprintf (dump_file, "SMS SC already optimized.\n");
+
+      ok = false;
+      goto clear;
+    }
+
+  if (dump_file)
+    {
+      fprintf (dump_file, "SMS Trying to optimize branch location\n");
+      fprintf (dump_file, "SMS partial schedule before trial:\n");
+      print_partial_schedule (ps, dump_file);
+    }
+
+  /* First, normalize the partial scheduling.  */
+  reset_sched_times (ps, amount);
+  rotate_partial_schedule (ps, amount);
+  if (dump_file)
+    {
+      fprintf (dump_file,
+	       "SMS partial schedule after normalization (ii, %d, SC %d):\n",
+	       ii, stage_count);
+      print_partial_schedule (ps, dump_file);
+    }
+
+  if (SMODULO (SCHED_TIME (g->closing_branch), ii) == ii - 1)
+    {
+      ok = true;
+      goto clear;
+    }
+
+  sbitmap_ones (sched_nodes);
+
+  /* Calculate the new placement of the branch.  It should be in row
+     ii-1 and fall into it's scheduling window.  */
+  if (get_sched_window (ps, g->closing_branch, sched_nodes, ii, &start,
+			&step, &end) == 0)
+    {
+      bool success;
+      ps_insn_ptr next_ps_i;
+      int branch_cycle = SCHED_TIME (g->closing_branch);
+      int row = SMODULO (branch_cycle, ps->ii);
+      int num_splits = 0;
+      sbitmap must_precede, must_follow, tmp_precede, tmp_follow;
+      int c;
+
+      if (dump_file)
+	fprintf (dump_file, "\nTrying to schedule node %d "
+		 "INSN = %d  in (%d .. %d) step %d\n",
+		 g->closing_branch->cuid,
+		 (INSN_UID (g->closing_branch->insn)), start, end, step);
+
+      gcc_assert ((step > 0 && start < end) || (step < 0 && start > end));
+      if (step == 1)
+	{
+	  c = start + ii - SMODULO (start, ii) - 1;
+	  gcc_assert (c >= start);
+	  if (c >= end)
+	    {
+	      ok = false;
+	      if (dump_file)
+		fprintf (dump_file,
+			 "SMS failed to schedule branch at cycle: %d\n", c);
+	      goto clear;
+	    }
+	}
+      else
+	{
+	  c = start - SMODULO (start, ii) - 1;
+	  gcc_assert (c <= start);
+
+	  if (c <= end)
+	    {
+	      if (dump_file)
+		fprintf (dump_file,
+			 "SMS failed to schedule branch at cycle: %d\n", c);
+	      ok = false;
+	      goto clear;
+	    }
+	}
+
+      must_precede = sbitmap_alloc (g->num_nodes);
+      must_follow = sbitmap_alloc (g->num_nodes);
+
+      /* Try to schedule the branch is it's new cycle.  */
+      calculate_must_precede_follow (g->closing_branch, start, end,
+				     step, ii, sched_nodes,
+				     must_precede, must_follow);
+
+      set_must_precede_follow (&tmp_follow, must_follow, &tmp_precede,
+			       must_precede, c, start, end, step);
+
+      /* Find the element in the partial schedule related to the closing
+         branch so we can remove it from it's current cycle.  */
+      for (next_ps_i = ps->rows[row];
+	   next_ps_i; next_ps_i = next_ps_i->next_in_row)
+	if (next_ps_i->node->cuid == g->closing_branch->cuid)
+	  break;
+
+      gcc_assert (next_ps_i);
+      gcc_assert (remove_node_from_ps (ps, next_ps_i));
+      success =
+	try_scheduling_node_in_cycle (ps, g->closing_branch,
+				      g->closing_branch->cuid, c,
+				      sched_nodes, &num_splits,
+				      tmp_precede, tmp_follow);
+      gcc_assert (num_splits == 0);
+      if (!success)
+	{
+	  if (dump_file)
+	    fprintf (dump_file,
+		     "SMS failed to schedule branch at cycle: %d, "
+		     "bringing it back to cycle %d\n", c, branch_cycle);
+
+	  /* The branch was failed to be placed in row ii - 1.
+	     Put it back in it's original place in the partial
+	     schedualing.  */
+	  set_must_precede_follow (&tmp_follow, must_follow, &tmp_precede,
+				   must_precede, branch_cycle, start, end,
+				   step);
+	  success =
+	    try_scheduling_node_in_cycle (ps, g->closing_branch,
+					  g->closing_branch->cuid,
+					  branch_cycle, sched_nodes,
+					  &num_splits, tmp_precede,
+					  tmp_follow);
+	  gcc_assert (success && (num_splits == 0));
+	  ok = false;
+	}
+      else
+	{
+	  /* The branch is placed in row ii - 1.  */
+	  if (dump_file)
+	    fprintf (dump_file,
+		     "SMS success in moving branch to cycle %d\n", c);
+
+	  update_node_sched_params (g->closing_branch, ii, c,
+				    PS_MIN_CYCLE (ps));
+	  ok = true;
+	}
+
+      free (must_precede);
+      free (must_follow);
+    }
+
+clear:
+  free (sched_nodes);
+  return ok;
+}
+
 static void
 duplicate_insns_of_cycles (partial_schedule_ptr ps, int from_stage,
 			   int to_stage, int for_prolog, rtx count_reg)
@@ -1116,6 +1340,7 @@ sms_schedule (void)
       int mii, rec_mii;
       unsigned stage_count = 0;
       HOST_WIDEST_INT loop_count = 0;
+      bool opt_sc_p = false;
 
       if (! (g = g_arr[loop->num]))
         continue;
@@ -1197,14 +1422,32 @@ sms_schedule (void)
       set_node_sched_params (g);
 
       ps = sms_schedule_by_order (g, mii, maxii, node_order);
-
-       if (ps)
-       {
-         stage_count = calculate_stage_count (ps);
-         gcc_assert(stage_count >= 1);
-         PS_STAGE_COUNT(ps) = stage_count;
-       }
-
+      
+      if (ps)
+	{
+	  /* Try to achieve optimized SC by normalizing the partial
+	     schedule (having the cycles start from cycle zero).
+	     The branch location must be placed in row ii-1 in the
+	     final scheduling.	If failed, shift all instructions to
+	     position the branch in row ii-1.  */
+	  opt_sc_p = optimize_sc (ps, g);
+	  if (opt_sc_p)
+	    stage_count = calculate_stage_count (ps, 0);
+	  else
+	    {
+	      /* Bring the branch to cycle ii-1.  */
+	      int amount = SCHED_TIME (g->closing_branch) - (ps->ii - 1);
+	      
+	      if (dump_file)
+		fprintf (dump_file, "SMS schedule branch at cycle ii-1\n");
+	      
+	      stage_count = calculate_stage_count (ps, amount);
+	    }
+	  
+	  gcc_assert (stage_count >= 1);
+	  PS_STAGE_COUNT (ps) = stage_count;
+	}
+      
       /* The default value of PARAM_SMS_MIN_SC is 2 as stage count of
 	 1 means that there is no interleaving between iterations thus
 	 we let the scheduling passes do the job in this case.  */
@@ -1225,12 +1468,16 @@ sms_schedule (void)
       else
 	{
 	  struct undo_replace_buff_elem *reg_move_replaces;
-          int amount = SCHED_TIME (g->closing_branch) + 1;
+
+          if (!opt_sc_p)
+            {
+	      /* Rotate the partial schedule to have the branch in row ii-1.  */
+              int amount = SCHED_TIME (g->closing_branch) - (ps->ii - 1);
+	      
+              reset_sched_times (ps, amount);
+              rotate_partial_schedule (ps, amount);
+            }
 	  
-	  /* Set the stage boundaries.	The closing_branch was scheduled
-	     and should appear in the last (ii-1) row.  */
-	  reset_sched_times (ps, amount);
-	  rotate_partial_schedule (ps, amount);
 	  set_columns_for_ps (ps);
 
 	  canon_loop (loop);
@@ -1382,13 +1629,11 @@ sms_schedule (void)
    scheduling window is empty and zero otherwise.  */
 
 static int
-get_sched_window (partial_schedule_ptr ps, int *nodes_order, int i,
+get_sched_window (partial_schedule_ptr ps, ddg_node_ptr u_node,
 		  sbitmap sched_nodes, int ii, int *start_p, int *step_p, int *end_p)
 {
   int start, step, end;
   ddg_edge_ptr e;
-  int u = nodes_order [i];
-  ddg_node_ptr u_node = &ps->g->nodes[u];
   sbitmap psp = sbitmap_alloc (ps->g->num_nodes);
   sbitmap pss = sbitmap_alloc (ps->g->num_nodes);
   sbitmap u_node_preds = NODE_PREDECESSORS (u_node);
@@ -1800,7 +2045,7 @@ sms_schedule_by_order (ddg_ptr g, int mi
 
 	  /* Try to get non-empty scheduling window.  */
 	 success = 0;
-         if (get_sched_window (ps, nodes_order, i, sched_nodes, ii, &start,
+         if (get_sched_window (ps, u_node, sched_nodes, ii, &start,
                                 &step, &end) == 0)
             {
               if (dump_file)
@@ -1817,24 +2062,11 @@ sms_schedule_by_order (ddg_ptr g, int mi
 
               for (c = start; c != end; c += step)
                 {
-                  sbitmap tmp_precede = NULL;
-                  sbitmap tmp_follow = NULL;
-
-                  if (c == start)
-                    {
-                      if (step == 1)
-                        tmp_precede = must_precede;
-                      else      /* step == -1.  */
-                        tmp_follow = must_follow;
-                    }
-                  if (c == end - step)
-                    {
-                      if (step == 1)
-                        tmp_follow = must_follow;
-                      else      /* step == -1.  */
-                        tmp_precede = must_precede;
-                    }
+		  sbitmap tmp_precede, tmp_follow;
 
+                  set_must_precede_follow (&tmp_follow, must_follow, 
+		                           &tmp_precede, must_precede, 
+                                           c, start, end, step);
                   success =
                     try_scheduling_node_in_cycle (ps, u_node, u, c,
                                                   sched_nodes,
@@ -2899,12 +3131,10 @@ ps_add_node_check_conflicts (partial_sch
 }
 
 /* Calculate the stage count of the partial schedule PS.  The calculation
-   takes into account the rotation to bring the closing branch to row
-   ii-1.  */
+   takes into account the rotation amount passed in ROTATION_AMOUNT.  */
 int
-calculate_stage_count (partial_schedule_ptr ps)
+calculate_stage_count (partial_schedule_ptr ps, int rotation_amount)
 {
-  int rotation_amount = (SCHED_TIME (ps->g->closing_branch)) + 1;
   int new_min_cycle = PS_MIN_CYCLE (ps) - rotation_amount;
   int new_max_cycle = PS_MAX_CYCLE (ps) - rotation_amount;
   int stage_count = CALC_STAGE_COUNT (-1, new_min_cycle, ps->ii);

Re: Patches ping

[Ayal Zaks]

>[PATCH, SMS 4/4] Misc. fixes
http://gcc.gnu.org/ml/gcc-patches/2011-05/msg01342.html
Sure, this is fine.
(Sorry for all the previous '?'s..).
Thanks,
Ayal.

2011/7/20 Revital Eres <revital.eres@linaro.org>
>
> Hello,
>
> [PATCH, SMS 3/4] Optimize stage count
> http://gcc.gnu.org/ml/gcc-patches/2011-05/msg01341.html
>
> [PATCH, SMS 4/4] Misc. fixes
> http://gcc.gnu.org/ml/gcc-patches/2011-05/msg01342.html
>
> [PATCH, SMS] Fix calculation of issue_rate
> http://gcc.gnu.org/ml/gcc-patches/2011-05/msg01344.html
>
> Thanks,
> Revital

Re: Patches ping

[Ayal Zaks]

> [PATCH, SMS] Fix calculation of issue_rate
> http://gcc.gnu.org/ml/gcc-patches/2011-05/msg01344.html

This is ok (with the updated Changelog). Alternatively, we can have a
local variable for holding the issue_rate.
Ayal.


2011/7/20 Revital Eres <revital.eres@linaro.org>:
> Hello,
>
> [PATCH, SMS 3/4] Optimize stage count
> http://gcc.gnu.org/ml/gcc-patches/2011-05/msg01341.html
>
> [PATCH, SMS 4/4] Misc. fixes
> http://gcc.gnu.org/ml/gcc-patches/2011-05/msg01342.html
>
> [PATCH, SMS] Fix calculation of issue_rate
> http://gcc.gnu.org/ml/gcc-patches/2011-05/msg01344.html
>
> Thanks,
> Revital
>

Patches ping

[Revital Eres]

Hello,

[PATCH, SMS 1/4] Fix calculation of row_rest_count
http://gcc.gnu.org/ml/gcc-patches/2011-05/msg01339.html

[PATCH, SMS 2/4] Move the creation of anti-dep edge
http://gcc.gnu.org/ml/gcc-patches/2011-05/msg01340.html

[PATCH, SMS 3/4] Optimize stage count
http://gcc.gnu.org/ml/gcc-patches/2011-05/msg01341.html

[PATCH, SMS 4/4] Misc. fixes
http://gcc.gnu.org/ml/gcc-patches/2011-05/msg01342.html

[PATCH, SMS] Fix calculation of issue_rate
http://gcc.gnu.org/ml/gcc-patches/2011-05/msg01344.html

Thanks,
Revital

Patches ping

[Revital Eres]

Hello,

http://gcc.gnu.org/ml/gcc-patches/2011-03/msg00350.html
- Support closing_branch_deps

http://gcc.gnu.org/ml/gcc-patches/2011-04/msg01309.html
- Support instructions with REG_INC_NOTE

http://gcc.gnu.org/ml/gcc-patches/2011-04/msg01294.html
- Avoid considering debug_insn when calculating SCCs

Thanks,
Revital

Patches ping

[Alexey Neyman]

Hi,

approved, uncommitted:
http://gcc.gnu.org/ml/gcc-patches/2005-03/msg01758.html

unreviewed:
http://gcc.gnu.org/ml/gcc-patches/2005-03/msg00027.html
http://gcc.gnu.org/ml/gcc-patches/2005-03/msg00140.html

Regards,
Alexey.

-- 
But wait!
This doesn't have to be the end!
                        -- Pkunks, SC2

Patches ping

[Alexey Neyman]

Hi,

approved, uncommitted:

  http://gcc.gnu.org/ml/gcc-patches/2005-03/msg00028.html

unreviewed:

  http://gcc.gnu.org/ml/gcc-patches/2005-03/msg00027.html
  http://gcc.gnu.org/ml/gcc-patches/2005-03/msg00140.html

Regards,
Alexey.

-- 
Two leaves pull water from the same root.
                        -- Supox, SC2

Re: Patches ping

[Paolo Bonzini]

Alexey Neyman wrote:
> Hi,
> 
> approved, uncommitted:
> 
>   http://gcc.gnu.org/ml/gcc-patches/2005-03/msg00028.html

Committed.

Paolo

Patches ping

[Alexey Neyman]

[-- Attachment #1: Type: text/plain, Size: 611 bytes --]

Hi,

This one was approved, but isn't committed:

  (update comments in calls.c)
  http://gcc.gnu.org/ml/gcc-patches/2005-03/msg00028.html

These patches are not reviewed still:

  (update to rtl.texi)
  http://gcc.gnu.org/ml/gcc-patches/2005-03/msg00027.html
 
  (update comment for DEF_BUILTIN)
  http://gcc.gnu.org/ml/gcc-patches/2005-03/msg00140.html

This patch was originally posted as 
http://gcc.gnu.org/ml/gcc-patches/2005-03/msg00736.html, its updated version 
is attached (fixed indentation, added a testcase). The description of the 
problem is available in the original message.

Regards,
Alexey.


[-- Attachment #2: libcpp.diff --]
[-- Type: text/x-diff, Size: 8643 bytes --]

2005-03-12  Alexey Neyman  <alex.neyman@auriga.ru>

	* libcpp/files.c (struct _cpp_file): New field 'this_file_dir'.
	(new_cpp_dir): New function (factored out from make_cpp_dir).
	(search_path_head): Cache values for "directory of this file"
	and "current working directory" search paths.
	(destroy_cpp_file): Free this_file_dir entry if it was allocated.
	(cpp_set_include_chains): Append a slash to the supplied paths and
	insert these paths in directory hash.
	* libcpp/internal.h (struct cpp_reader): New field 'cwd'.


2005-03-12  Alexey Neyman  <alex.neyman@auriga.ru>

	* gcc.dg/cpp/include_next-1.c: New test.
	* gcc.dg/cpp/include_next-1.h, gcc.dg/cpp/include_next-1a.h,
	  gcc.dg/cpp/nextdir/include_next-1a.h: New files.

*** gcc.orig/libcpp/files.c	2005-03-11 18:10:59.000000000 +0300
--- gcc.work/libcpp/files.c	2005-03-09 11:44:26.000000000 +0300
***************
*** 79,84 ****
--- 79,88 ----
       header.  */
    cpp_dir *dir;
  
+   /* The directory of this file, filled if this file includes anything
+      in quotes.  */
+   cpp_dir *this_file_dir;
+ 
    /* As filled in by stat(2) for the file.  */
    struct stat st;
  
***************
*** 162,167 ****
--- 166,172 ----
  					     const cpp_dir *start_dir);
  static _cpp_file *make_cpp_file (cpp_reader *, cpp_dir *, const char *fname);
  static void destroy_cpp_file (_cpp_file *);
+ static cpp_dir *new_cpp_dir (const char *dir_name, cpp_dir *next, int sysp);
  static cpp_dir *make_cpp_dir (cpp_reader *, const char *dir_name, int sysp);
  static void allocate_file_hash_entries (cpp_reader *pfile);
  static struct file_hash_entry *new_file_hash_entry (cpp_reader *pfile);
***************
*** 772,778 ****
  search_path_head (cpp_reader *pfile, const char *fname, int angle_brackets,
  		  enum include_type type)
  {
!   cpp_dir *dir;
    _cpp_file *file;
  
    if (IS_ABSOLUTE_PATH (fname))
--- 777,783 ----
  search_path_head (cpp_reader *pfile, const char *fname, int angle_brackets,
  		  enum include_type type)
  {
!   cpp_dir *dir = NULL;
    _cpp_file *file;
  
    if (IS_ABSOLUTE_PATH (fname))
***************
*** 785,802 ****
       the current file was found, but if it was found via an absolute
       path use the normal search logic.  */
    if (type == IT_INCLUDE_NEXT && file->dir)
!     dir = file->dir->next;
    else if (angle_brackets)
      dir = pfile->bracket_include;
    else if (type == IT_CMDLINE)
!     /* -include and -imacros use the #include "" chain with the
         preprocessor's cwd prepended.  */
!     return make_cpp_dir (pfile, "./", false);
    else if (pfile->quote_ignores_source_dir)
      dir = pfile->quote_include;
    else
!     return make_cpp_dir (pfile, dir_name_of_file (file),
! 			 pfile->buffer ? pfile->buffer->sysp : 0);
  
    if (dir == NULL)
      cpp_error (pfile, CPP_DL_ERROR,
--- 790,841 ----
       the current file was found, but if it was found via an absolute
       path use the normal search logic.  */
    if (type == IT_INCLUDE_NEXT && file->dir)
!     {
!       struct file_hash_entry *entry, **hash_slot;
! 
!       /* It should always be in hash.  However, we may find not the same
! 	 entry if the current file was included in quotes.  */
!       hash_slot = (struct file_hash_entry **)
! 	htab_find_slot_with_hash (pfile->dir_hash, file->dir->name,
! 				  htab_hash_string (file->dir->name),
! 				  INSERT);
!       for (entry = *hash_slot; entry; entry = entry->next)
! 	if (entry->start_dir == NULL)
! 	  {
! 	    dir = entry->u.dir->next;
! 	    break;
! 	  }
! 
!       if (!dir)
! 	cpp_error (pfile, CPP_DL_ICE, "No hashed entry for include_next");
!     }
    else if (angle_brackets)
      dir = pfile->bracket_include;
    else if (type == IT_CMDLINE)
!     {
!       /* -include and -imacros use the #include "" chain with the
         preprocessor's cwd prepended.  */
!       if (!pfile->cwd)
! 	{
! 	  /* Hash it if it wasn't.  */
! 	  pfile->cwd = make_cpp_dir (pfile, "./", false);
! 	}
!       return pfile->cwd;
!     }
    else if (pfile->quote_ignores_source_dir)
      dir = pfile->quote_include;
    else
!     {
!       if (!file->this_file_dir)
! 	{
! 	  dir = make_cpp_dir (pfile, dir_name_of_file (file),
! 			      pfile->buffer ? pfile->buffer->sysp : 0);
! 	  /* Make a copy that is always before all includes.  */
! 	  file->this_file_dir = new_cpp_dir (dir->name, pfile->quote_include,
! 					     dir->sysp);
! 	}
!       return file->this_file_dir;
!     }
  
    if (dir == NULL)
      cpp_error (pfile, CPP_DL_ERROR,
***************
*** 910,919 ****
--- 949,975 ----
  {
    if (file->buffer)
      free ((void *) file->buffer);
+   if (file->this_file_dir)
+     free ((void *) file->this_file_dir);
    free ((void *) file->name);
    free (file);
  }
  
+ /* Create a new cpp_dir entry.  */
+ static cpp_dir *
+ new_cpp_dir (const char *dir_name, cpp_dir *next, int sysp)
+ {
+   cpp_dir *dir;
+   
+   dir = xcalloc (1, sizeof (cpp_dir));
+   dir->next = next;
+   dir->name = (char *) dir_name;
+   dir->len = strlen (dir_name);
+   dir->sysp = sysp;
+   dir->construct = 0;
+   return dir;
+ }
+ 
  /* A hash of directory names.  The directory names are the path names
     of files which contain a #include "", the included file name is
     appended to this directories.
***************
*** 937,948 ****
      if (entry->start_dir == NULL)
        return entry->u.dir;
  
!   dir = xcalloc (1, sizeof (cpp_dir));
!   dir->next = pfile->quote_include;
!   dir->name = (char *) dir_name;
!   dir->len = strlen (dir_name);
!   dir->sysp = sysp;
!   dir->construct = 0;
  
    /* Store this new result in the hash table.  */
    entry = new_file_hash_entry (pfile);
--- 993,999 ----
      if (entry->start_dir == NULL)
        return entry->u.dir;
  
!   dir = new_cpp_dir(dir_name, pfile->quote_include, sysp);
  
    /* Store this new result in the hash table.  */
    entry = new_file_hash_entry (pfile);
***************
*** 1180,1185 ****
--- 1231,1239 ----
  cpp_set_include_chains (cpp_reader *pfile, cpp_dir *quote, cpp_dir *bracket,
  			int quote_ignores_source_dir)
  {
+   char *tmp;
+   struct file_hash_entry **hash_slot, *entry;
+ 
    pfile->quote_include = quote;
    pfile->bracket_include = quote;
    pfile->quote_ignores_source_dir = quote_ignores_source_dir;
***************
*** 1188,1193 ****
--- 1242,1261 ----
      {
        quote->name_map = NULL;
        quote->len = strlen (quote->name);
+       tmp = quote->name;
+       quote->name = xcalloc(1, quote->len + 2);
+       sprintf(quote->name, "%s/", tmp);
+       free(tmp);
+       quote->len++;
+       hash_slot = (struct file_hash_entry **)
+ 		  htab_find_slot_with_hash (pfile->dir_hash, quote->name,
+ 					    htab_hash_string (quote->name),
+ 					    INSERT);
+       entry = new_file_hash_entry (pfile);
+       entry->next = *hash_slot;
+       entry->start_dir = NULL;
+       entry->u.dir = quote;
+       *hash_slot = entry;
        if (quote == bracket)
  	pfile->bracket_include = bracket;
      }
*** gcc.orig/libcpp/internal.h	2005-02-14 17:43:56.000000000 +0300
--- gcc.work/libcpp/internal.h	2005-03-09 11:41:30.000000000 +0300
***************
*** 333,338 ****
--- 333,339 ----
    cpp_token directive_result;
  
    /* Search paths for include files.  */
+   struct cpp_dir *cwd;			/* Current working directory */
    struct cpp_dir *quote_include;	/* "" */
    struct cpp_dir *bracket_include;	/* <> */
    struct cpp_dir no_search_path;	/* No path.  */
*** /dev/null	2004-11-24 12:27:29.000000000 +0300
--- gcc.work/gcc/testsuite/gcc.dg/cpp/include_next-1.h	2005-03-11 17:55:20.000000000 +0300
***************
*** 0 ****
--- 1,2 ----
+ /* This file is used by the include_next-1.c test case.  */
+ #include_next <include_next-1a.h>
*** /dev/null	2004-11-24 12:27:29.000000000 +0300
--- gcc.work/gcc/testsuite/gcc.dg/cpp/include_next-1.c	2005-03-11 18:09:40.000000000 +0300
***************
*** 0 ****
--- 1,3 ----
+ /* { dg-do compile } */
+ /* { dg-options "-isystem ${srcdir}/gcc.dg/cpp -isystem ${srcdir}/gcc.dg/cpp/nextdir" } */
+ #include "include_next-1.h"
*** /dev/null	2004-11-24 12:27:29.000000000 +0300
--- gcc.work/gcc/testsuite/gcc.dg/cpp/nextdir/include_next-1a.h	2005-03-11 17:57:24.000000000 +0300
***************
*** 0 ****
--- 1,2 ----
+ /* This file is used by the include_next-1.c test case.  */
+ /* This file is empty, including it is ok.  */
*** /dev/null	2004-11-24 12:27:29.000000000 +0300
--- gcc.work/gcc/testsuite/gcc.dg/cpp/include_next-1a.h	2005-03-11 17:57:44.000000000 +0300
***************
*** 0 ****
--- 1,2 ----
+ /* This file is used by the include_next-1.c test case.  */
+ #error "Wrong file included"

Patches ping

[Alexey Neyman]

Hi,

These two patches were approved but are not committed yet (I have no commit 
privileges myself):

http://gcc.gnu.org/ml/gcc-patches/2005-03/msg00024.html
http://gcc.gnu.org/ml/gcc-patches/2005-03/msg00028.html

These two patches are not reviewed still:

http://gcc.gnu.org/ml/gcc-patches/2005-03/msg00027.html
http://gcc.gnu.org/ml/gcc-patches/2005-03/msg00140.html

Thanks,
Alexey.

Patches ping...

[Alexey Neyman]

Hi,

[is this right list for doc patches?]

Update comments after ECF_LONGJMP removal:
http://gcc.gnu.org/ml/gcc-patches/2004-11/msg02258.html

Update stale information in rtl.texi:
http://gcc.gnu.org/ml/gcc-patches/2004-12/msg00584.html

Regards,
Alexey.

-- 
Of course, as a loving parent I must push you from the nest.
                        -- Pkunks, SC2

Re: Patches ping...

[Gerald Pfeifer]

On Mon, 20 Dec 2004, Alexey Neyman wrote:
> [is this right list for doc patches?]

Yes, it is.

> Update comments after ECF_LONGJMP removal:
> http://gcc.gnu.org/ml/gcc-patches/2004-11/msg02258.html
> 
> Update stale information in rtl.texi:
> http://gcc.gnu.org/ml/gcc-patches/2004-12/msg00584.html

These patches, however, need to be be approved by code maintainers.

Gerald

Patches ping

[H. J. Lu]

Could someone please take a look at

http://gcc.gnu.org/ml/gcc-patches/2004-11/msg01352.html
http://gcc.gnu.org/ml/gcc-patches/2004-11/msg01454.html
http://gcc.gnu.org/ml/gcc-patches/2004-11/msg01456.html
http://gcc.gnu.org/ml/gcc-patches/2004-10/msg02209.html

Thanks.


H.J.

Patches ping

[Zdenek Dvorak]

Hello,

there are some patches pending for several serious issues
with the loop optimizer:

http://gcc.gnu.org/ml/gcc-patches/2004-09/msg01120.html
  -- patch for PR 17133

http://gcc.gnu.org/ml/gcc-patches/2004-09/msg02910.html
  -- cfg cleanup inside loop optimizer

http://gcc.gnu.org/ml/gcc-patches/2004-09/msg02937.html
  -- really consider "important" candidates in ivopts

Zdenek