[tree-ssa]: Still a problem with insertion, this time, after

[Andrew MacLeod]

On Sun, 2003-06-01 at 20:39, Daniel Berlin wrote:
> Insertion after a LOOP_EXPR at the end of the bb seems not to work.
> You end up with a BB that looks like:
> (gdb) p debug_tree_bb (bb)
> BLOCK       0
> PRED:       ENTRY
> SUCC:       1
> PARENT:     nil
> LOOP DEPTH: 0
> NEXT BLOCK: 1
> PREV BLOCK: -1
> 23  while (1)
> -1  pretmp.4_24 = in_9 + 1B
> 


Ahhh. This was something that once upon a time was discussed I think.
Once upon a time, the insert routines were going to be super smart and
do lots of majik. Super smart routines were punted on because there were
a number of ambiguities about what the client might want, and creating
new basic blocks willy-nilly causes numberous algorithms to not work
properly. So thats is left to the person doing the inserting to decide
what to do, and the plan is to have the more complex routines (for which
the prototypes only exist  :-( )

/* Stmt list insertion routines.  */

extern void bsi_insert_list_before      PARAMS ((block_stmt_iterator *, tree_stmt_anchor));
extern void bsi_insert_list_after       PARAMS ((block_stmt_iterator *, tree_stmt_anchor));
extern block_stmt_iterator bsi_insert_list_on_edge      PARAMS ((edge, tree_stmt_anchor));


These routines were going to have the smarts to make whatever basic
block modifications were necessary in order to insert a loop node, or an
arbitrary sequence of stmts which resulted in flow changes.

As a result of that, the current routines for inserting after and before
stmts do not look at the stmt being inserted before/after nor the
current one in order to create new basic block determinations. (except
for splitting critical edges when so committed).

So I'm guessing what you are doing is bsi_insert_after() the LOOP stmt,
which is the end of a basic block. This results in an incorrectly formed
block.  The stmt you are inserting should actually go at the beginning
of the next block after the loop stmt. (Clarification for other
readers.. The body of the loop is nested under the LOOP_EXPR stmt. The
'next' stmt after the loop stmt is normally the first stmt after the end
of the loop.)
  
Part of the reason that approach exists at the moment is due to
ambiguity (at 2 levels). If you insert after a loop stmt, are you trying
to insert before the first stmt in the loop, or after the last?  I would
say the latter, but there could be other opinions. (Same hold true for
the IF stmt.)

The other question is exactly what are you trying to do. Simply
inserting a stmt after a LOOP_EXPR will only cause it to be executed
when the loop falls out the bottom, which it never does. The next stmt
after the loop stmt is usually a label which is the exit target of the
loop. So your stmt would actually need to be inserted after the label,
(so 2 stmt's later)  not the loop_stmt.  Maybe other code has already
been inserted between the end of the loop_stmt and its label. So thats
not always the right place either.   See where Im going? Insertions
around these container stmts require the person doing them to now what
they are trying to do... So right now the insert routines punt.

I'd hazzard a guess that if you are trying to insert something that
comes right after the loop stmt, you probably need to insert before the
first executable stmt in the succesor block of a block in the loop which
exits. (or at the end of that block if there are no executable stmts).

Perhaps a helper routine or two to identify these common spots for LOOP,
COND and SWITCH would be appropriate.  ie
  bsi_insert_after_loop_finish ()
  bsi_insert_start_then ()
  bsi_insert_start_else ()
  bsi_insert_after_if ()
etc.

If you are trying to insert something on the backedge of the loop, then
you want to do a bsi_insert_after() on the last stmt in the LOOP_BODY
chain...  

> Something isn't updated right here because the reverse iterator gets 
> one statement, and the forward iterator gets  two:
> 2480    count_stmts_in_bb (bb)
> 2481         basic_block bb;
> 2482    {
> 2483      block_stmt_iterator bsi;
> 2484      int num_stmt1 = 0;
> 2485      int num_stmt2 = 0;
> 2486
> 2487      bsi = bsi_start (bb);
> 2488      for (; !bsi_end_p (bsi); bsi_next  (&bsi))
> 2489        num_stmt1++;
> (gdb) l
> 2490
> 2491      bsi = bsi_last (bb);
> 2492      for (; !bsi_end_p (bsi); bsi_prev  (&bsi))
> 2493        num_stmt2++;
> 2494      if (num_stmt1 != num_stmt2)
> 2495        abort ();
> 2496      return num_stmt1;
> 2497    }
> 
> We abort on line 2495.


After all that, however, we still shouldnt have this problem. 

Is there *anything* after the while() stmt? Or does the function end
there?  (ir, the LOOP stmt is the last stmt in the program? I think it
looks like we might be missing the case of updating the block ending
pointer in that case, maybe.

However, you are going to require the creation of a new basic block in
this instance... 

Andrew