Re: [patch] and question on loops state in the vectorizer

[Dorit Naishlos <DORIT at il dot ibm dot com>]

Zdenek Dvorak <rakdver@atrey.karlin.mff.cuni.cz> wrote on 20/02/2005
22:35:29:
> Hello,
>
> > While on the issue of loop entry/exit edges and such (in the
vectorizer):
> >
> > (1) (patch): We should call flow_loop_scan after loop_split_edge, to
update
> > preheader and entry-edges (this is after loop peeling may have taken
> > place). Bootstrapped (with vectorization enabled) and tested (also on
SPEC)
> > on powerpc-darwin.
>
> you should not use flow_loop_scan at all.

but loop->entry_edges is not updated after we, say, split the preheader
edge.
but, as you note below, if we don't use this API, and instead use
loop_preheader_edge (and loop->single_exit), which we do, this shouldn't be
a problem.

> There is always just a single
> entry edge (loop_preheader_edge (loop)) and just a single preheader
> block (loop_preheader_edge (loop)->src).  As for exit edges, I would
> recommend using either loop->single_exit if this is sufficient (I am
> fairly sure vectorizer works only with single exit loops, so it is),
> or get_loop_exit_edges.
>

so looks like I can drop the first patch.
or rather instead, remove the call to loop_split_edge which was there to
guarantee that the pre_header bb has a single successor; this is not needed
cause we don't add stmts on the pre_header bb anymore (we always add stmts
on the preheader_edge using insert_on_edge). Yet to be bootstrapped and
tested:

Index: tree-vect-transform.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/tree-vect-transform.c,v
retrieving revision 2.1
diff -c -3 -p -r2.1 tree-vect-transform.c
*** tree-vect-transform.c       17 Feb 2005 08:47:28 -0000      2.1
--- tree-vect-transform.c       20 Feb 2005 21:02:11 -0000
*************** vect_transform_loop (loop_vec_info loop_
*** 1678,1691 ****
      ratio = build_int_cst (TREE_TYPE (LOOP_VINFO_NITERS (loop_vinfo)),
                LOOP_VINFO_INT_NITERS (loop_vinfo) / vectorization_factor);

!   /* 1) Make sure the loop header has exactly two entries
!      2) Make sure we have a preheader basic block.  */
!
    gcc_assert (EDGE_COUNT (loop->header->preds) == 2);

-   loop_split_edge_with (loop_preheader_edge (loop), NULL);
-
-
    /* FORNOW: the vectorizer supports only loops which body consist
       of one basic block (header + empty latch). When the vectorizer will
       support more involved loop forms, the order by which the BBs are
--- 1678,1686 ----
      ratio = build_int_cst (TREE_TYPE (LOOP_VINFO_NITERS (loop_vinfo)),
                LOOP_VINFO_INT_NITERS (loop_vinfo) / vectorization_factor);

!   /* Make sure the loop header has exactly two entries  */
    gcc_assert (EDGE_COUNT (loop->header->preds) == 2);

    /* FORNOW: the vectorizer supports only loops which body consist
       of one basic block (header + empty latch). When the vectorizer will
       support more involved loop forms, the order by which the BBs are


> > (2) (question:) On entry to vectorization pass we assume that the loop
> > state (entry/exit edges etc.) are up to date. i.e. - we don't call
> > flow_loop_scan on the loops before we analyze them. Is that a safe
> > assumption?
>
> No.
>

In this case, we need the following patch (yet to be bootstrapped and
tested):

      * tree-vectorizer.c (vectorize_loops): Call flow_loop_scan and
      verify_loop_structure.

Index: tree-vectorizer.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/tree-vectorizer.c,v
retrieving revision 2.75
diff -c -3 -p -r2.75 tree-vectorizer.c
*** tree-vectorizer.c   17 Feb 2005 16:19:49 -0000      2.75
--- tree-vectorizer.c   20 Feb 2005 14:25:21 -0000
*************** vectorize_loops (struct loops *loops)
*** 1577,1582 ****
--- 1577,1593 ----
    verify_loop_closed_ssa ();
  #endif

+
+   for (i = 1; i < loops->num; i++)
+     flow_loop_scan (loops->parray[i], LOOP_ALL);
+
+ #ifdef ENABLE_CHECKING
+   gcc_assert (loops->state & LOOPS_HAVE_MARKED_SINGLE_EXITS);
+   gcc_assert (loops->state & LOOPS_HAVE_PREHEADERS);
+   gcc_assert (loops->state & LOOPS_HAVE_SIMPLE_LATCHES);
+   verify_loop_structure (loops);
+ #endif
+
    compute_immediate_uses (TDFA_USE_OPS, need_imm_uses_for);

    /*  ----------- Analyze loops. -----------  */


> > Also, I tested the following right at the entry to vectorize_loops:
> > + #ifdef ENABLE_CHECKING
> > +   gcc_assert (loops->state & LOOPS_HAVE_MARKED_SINGLE_EXITS);
> > +   gcc_assert (loops->state & LOOPS_HAVE_PREHEADERS);
> > +   gcc_assert (loops->state & LOOPS_HAVE_SIMPLE_LATCHES);
> > +   verify_loop_structure (loops);
> > + #endif
> > +
> > The above should hold right after loop init, but I wasn't able to
convince
> > myself that it's also guaranteed to be the case just before the
vectorizer.
> > These asserts do hold through SPEC and bootstrap with vectorization
enabled
> > FWIW. So the question is - is it safe to assume the above? ok to add
these
> > asserts?
>
> Yes.  loops->state flags are never reset (except if you do it
> explicitly), and are checked in verify_loop_structure.
>

thanks,

dorit


> Zdenek