[PATCH][TUPLES] Two simple bug fixes.

[Richard Guenther]

On Sun, Apr 13, 2008 at 11:46 AM, Doug Kwan (關振德) <dougkwan@google.com> wrote:
> We can add a check to make sure that the statement pointed to by the
>  gsi indeed have the bb set correctly but we already have a CFG
>  consistency check in the tuples branch and that's how this bug got
>  caught.  Do we still need another check?

Maybe I'm missing sth, but

>  >  >   -  gimple_stmt_iterator last = gsi_last (bb_seq (bb));
>  >  >   +  gimple_stmt_iterator last = gsi_last_bb (bb);

this doesn't look CFG checking related.  If the above change makes any
difference
then the returned gsi has to be different in some cases.  Which is what I asked
to check for in gsi_last.  I don't know any of bb_seq or gsi_last or gsi_last_bb
from their implementation, so I have no idea what the actual case was you ran
into that made that difference - but it would be nice to catch it early.

Richard.

>  -Doug
>
>  2008/4/13 Richard Guenther <richard.guenther@gmail.com>:
>
>
> > 2008/4/13 Doug Kwan (關振德) <dougkwan@google.com>:
>  >
>  > > Here is the patch.
>  >  >
>  >  >  -Doug
>  >  >
>  >  >
>  >  >
>  >  >   2008-04-13  Doug Kwan  <dougkwan@google.com>
>  >  >
>  >  >         * gimple-iterator.c (gsi_move_to_bb_end): Use gsi_last_bb
>  >  >         instead of calling both gsi_last and bb_seq.
>  >  >         * passes.c (init_optimization_passes): Re-eanble second tail-recursion
>  >  >         pass.
>  >  >
>  >  >   Index: gcc/gcc/gimple-iterator.c
>  >  >   ===================================================================
>  >  >   --- gcc/gcc/gimple-iterator.c   (revision 134208)
>  >  >   +++ gcc/gcc/gimple-iterator.c   (working copy)
>  >  >   @@ -566,7 +566,7 @@ gsi_move_before (gimple_stmt_iterator *f
>  >  >   void
>  >  >   gsi_move_to_bb_end (gimple_stmt_iterator *from, basic_block bb)
>  >  >   {
>  >  >   -  gimple_stmt_iterator last = gsi_last (bb_seq (bb));
>  >  >   +  gimple_stmt_iterator last = gsi_last_bb (bb);
>  >
>  >  Can we put an assert guarded by #if ENABLE_CHECKING for the case you ran
>  >  into in gsi_last?  I also hit similar issues if you split a bb when
>  >  there are active
>  >  tsis in that bb (they get invalid bbs and strange things occur).
>  >
>  >  Otherwise this is ok.
>  >
>  >  Richard.
>  >
>  >
>  >
>  >  >    /* Have to check gsi_end_p because it could be an empty block.  */
>  >  >    if (!gsi_end_p (last) && is_ctrl_stmt (gsi_stmt (last)))
>  >  >   Index: gcc/gcc/passes.c
>  >  >   ===================================================================
>  >  >   --- gcc/gcc/passes.c    (revision 134209)
>  >  >   +++ gcc/gcc/passes.c    (working copy)
>  >  >   @@ -629,8 +629,8 @@ init_optimization_passes (void)
>  >  >        NEXT_PASS (pass_phi_only_cprop);
>  >  >        NEXT_PASS (pass_tree_ifcombine);
>  >  >        NEXT_PASS (pass_phiopt);
>  >  >   -      NEXT_PASS (pass_tail_recursion);
>  >  >   #endif
>  >  >   +      NEXT_PASS (pass_tail_recursion);
>  >  >        NEXT_PASS (pass_ch);
>  >  >   #if 0
>  >  >        NEXT_PASS (pass_stdarg);
>  >  >
>  >  >
>  >  >   2008/4/13 Rafael Espindola <espindola@google.com>:
>  >  >
>  >  >
>  >  >  > Patch is missing.
>  >  >   >
>  >  >   >  2008/4/13 Doug Kwan (關振德) <dougkwan@google.com>:
>  >  >
>  >  >
>  >  >  >
>  >  >   >
>  >  >   > > Hi Diego,
>  >  >   >  >
>  >  >   >  >     Could you please review this simple patch?  The first fix is in
>  >  >   >  >  gsi_move_to_bb_end.  The bug is quite subtle.  In the old code,  if
>  >  >   >  >  the target bb is empty, bb_seq will be empty and gsi_last will not
>  >  >   >  >  have the correct bb set.  That causes the moved gimple to have NULL is
>  >  >   >  >  its bb.  The second fix is to re-enable the second tail recursion
>  >  >   >  >  pass.  I did not do so when I check in the patch for tree-tailcall.c.
>  >  >   >  >
>  >  >   >  >  Thanks.
>  >  >   >  >
>  >  >   >  >  -Doug
>  >  >   >  >
>  >  >   >
>  >  >   >
>  >  >   >
>  >  >   >  --
>  >  >   >  Rafael Avila de Espindola
>  >  >   >
>  >  >   >  Google Ireland Ltd.
>  >  >   >  Gordon House
>  >  >   >  Barrow Street
>  >  >   >  Dublin 4
>  >  >   >  Ireland
>  >  >   >
>  >  >   >  Registered in Dublin, Ireland
>  >  >   >  Registration Number: 368047
>  >  >   >
>  >  >
>  >
>