Re: [PATCH] FOR_EACH_RTX construct to replace for_each_rtx

[Ian Lance Taylor <ian at airs dot com>]

Paolo Bonzini <paolo.bonzini@lu.unisi.ch> writes:

> + /* Traverse X via depth-first search, calling F for each
> +    sub-expression (including X itself).  F is also passed the DATA.
> +    If F returns -1, do not traverse sub-expressions, but continue
> +    traversing the rest of the tree.  Upon an iteration, SUB_RTX points
> +    to an rtx in the tree; rtx_iter_skip can be called to avoid visiting
> +    the subexpressions of SUB_RTX.  */
> + 
> + /* Always ensure two free items in the list, which are needed for rtvecs.
> +    The ENABLE_CHECKING version stores each element in the NEXT field of
> +    the iterator, so that rtx_iter_skip can check that the correct value
> +    is passed.  */
> + #define FOR_EACH_RTX(X, SUB_RTX, ITER) \
> +   for ((SUB_RTX) = rtx_iter_init (&(ITER), (X)); \
> +        (SUB_RTX); \
> +        ((ITER).free == NULL \
> + 	? (void) ((ITER).free = alloca (sizeof (rtx_stack_entry)), \
> + 	    (ITER).free->next = NULL) : (void) 0), \
> +        ((ITER).free->next == NULL \
> + 	? (void) ((ITER).free->next = alloca (sizeof (rtx_stack_entry)), \
> + 	    (ITER).free->next->next = NULL) : (void) 0), \
> +         (SUB_RTX) = rtx_iter_next (&(ITER)))

The first comment seems wrong.  It seems like a copy of the comment
before the for_each_rtx() function, but in the macro there is no F.

For that matter, I don't see the ENABLE_CHECKING version described in
the second comment.


Since you're bumming clock cycles here, I think you should consider
that each alloca() call is not free.  On many systems it will generate
code which aligns the stack pointer.  Suppose you do something along
the lines of

#define RTX_STACK_ENTRIES (32)

struct rtx_stack
{
  int next;
  struct rtx_stack *previous;
  struct rtx_stack_entry[RTX_STACK_ENTRIES] stack;
};

// rtx_iter includes an rtx_stack element

#define FOR_EACH_RTX(X, SUB_RTX, ITER)
  for ((SUB_RTX) = rtx_iter_init (&(ITER), (X));
       (SUB_RTX);
       (((ITER).next == RTX_STACK_ENTRIES
         && (ITER).next_stack == NULL)
        ? rtx_add_stack (alloca (sizeof (rtx_stack)))
        : 0),
       (SUB_RTX) = rtx_iter_next (&(ITER)))

With of course the obvious adjustments to rtx_iter_init() and
rtx_iter_next().


Incidentally, an alternative approach might be something more STL
like:

rtx_iter iter;
rtx *sub;

for (rtx_iter_begin (x, &iter); 
     !rtx_iter_done (&iter);
     rtx_iter_next (&iter))
  {
    rtx *sub;

    sub = rtx_iter_here (iter);
    // Optional rtx_skip_iter_children (iter)
  }

Ian