[gomp] walk_tree and OMP_CLAUSE_* (take 2)

[Jakub Jelinek <jakub at redhat dot com>]

On Sat, Nov 05, 2005 at 03:10:48PM -0800, Richard Henderson wrote:
> On Sat, Nov 05, 2005 at 03:36:12PM -0500, Jakub Jelinek wrote:
> >        if (len)
> >  	{
> > -	  /* The common case is that we may tail recurse here.  */
> > -	  if (code != BIND_EXPR
> > -	      && !TREE_CHAIN (*tp))
> > -	    WALK_SUBTREE_TAIL (TREE_OPERAND (*tp, len - 1));
> > +	  if (code >= OMP_CLAUSE_PRIVATE && code <= OMP_CLAUSE_DEFAULT)
> > +	    {
> > +	      WALK_SUBTREE (TREE_OPERAND (*tp, len - 1));
> > +	      WALK_SUBTREE_TAIL (OMP_CLAUSE_CHAIN (*tp));
> > +	    }
> >  	  else
> > -	    WALK_SUBTREE (TREE_OPERAND (*tp, len - 1));
> > +	    WALK_SUBTREE_TAIL (TREE_OPERAND (*tp, len - 1));
> >  	}
> >  #endif
> >      }
> 
> This one's wrong, afaics, since it doesn't run the chain
> of the zero length clauses like nowait.

Ok.
I benchmarked a little bit the following two patches on x86_64-linux
(--enable-checking=release, bootstrapped) on insn-attrtab.c.
gimplify.c has been slightly modified to amplify walk_tree timing:
--- gimplify.c.jj6      2005-11-07 22:28:14.000000000 +0100
+++ gimplify.c  2005-11-08 08:51:36.000000000 +0100
@@ -822,6 +822,13 @@ unmark_visited_r (tree *tp, int *walk_su
   return NULL_TREE;
 }

+static tree
+dummy_r (tree *tp ATTRIBUTE_UNUSED, int *walk_subtrees ATTRIBUTE_UNUSED,
+                 void *data ATTRIBUTE_UNUSED)
+{
+  return NULL_TREE;
+}
+
 /* Unshare all the trees in BODY_P, a pointer into the body of FNDECL, and the
    bodies of any nested functions if we are unsharing the entire body of
    FNDECL.  */
@@ -830,7 +837,10 @@ static void
 unshare_body (tree *body_p, tree fndecl)
 {
   struct cgraph_node *cgn = cgraph_node (fndecl);
+int i;

+for (i = 0; i < 5000; i++)
+walk_tree (body_p, dummy_r, NULL, NULL);
   walk_tree (body_p, copy_if_shared_r, NULL, NULL);
   if (body_p == &DECL_SAVED_TREE (fndecl))
     for (cgn = cgn->nested; cgn; cgn = cgn->next_nested)

The numbers I got are:
vanilla:
user    2m20.044s
user    2m20.656s
user    2m20.328s
user    2m20.717s
user    2m20.719s
user    2m20.627s
simple.patch:
user    2m22.343s
user    2m22.437s
user    2m22.252s
user    2m22.651s
user    2m22.808s
user    2m22.451s
switch.patch:
user    2m21.333s
user    2m20.881s
user    2m21.170s
user    2m20.908s
user    2m20.883s
user    2m21.243s

switch.patch is Richard's patch with tiny modifications from me
(handling OMP_CLAUSE_REDUCTION separately, as it now has 4 arguments,
and handling TARGET_EXPR separately in the switch as well, to avoid
an extra if in the fast path).

User time of vanilla without the gimplify.c change is:
user    0m56.927s
user    0m56.964s

which means vanilla GCC spents around 16.71ms on walk_tree calls from
unchanged unshare_body if copy_if_shared_r was a dummy procedure and
with Richard's patch around 16.82ms.

Is the switch.patch ok for gomp or should I work on an alternate solution?
That would need (small) change to all frontends, as the number of tree codes
differs between each frontend.

	Jakub

Attachment: simple.patch
Description: Text document

Attachment: switch.patch
Description: Text document