This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] Fix cond_exec_find_if_block (PR rtl-optimization/56745)


On Tue, Apr 02, 2013 at 02:57:13PM +0200, Richard Biener wrote:
> On Tue, 2 Apr 2013, Jakub Jelinek wrote:
> > On the (undefined behavior) testcase below, we end up with
> > then_bb ending with __builtin_unreachable () at the tree level, therefore
> > no successor at the RTL level, and else_bb being EXIT_BLOCK_PTR (i.e.
> > conditional return before a bb with undefined behavior at the end).
> > Trying to optimize that into a conditional execution of the then_bb insns
> > doesn't work, we can't merge the else_bb with then_bb and test_bb in this
> > case, plus it doesn't look like something that would be desirable to do
> > (conditional return is surely better).
> > 
> > Fixed thusly, ok for trunk/4.8?
> 
> I wonder if
> 
>   /* Make sure IF, THEN, and ELSE, blocks are adjacent.  Actually, we get 
> the
>      first condition for free, since we've already asserted that there's a
>      fallthru edge from IF to THEN.  Likewise for the && and || blocks, 
> since
>      we checked the FALLTHRU flag, those are already adjacent to the last 
> IF
>      block.  */
>   /* ??? As an enhancement, move the ELSE block.  Have to deal with
>      BLOCK notes, if by no other means than backing out the merge if they
>      exist.  Sticky enough I don't want to think about it now.  */
>   next = then_bb;
>   if (else_bb && (next = next->next_bb) != else_bb)
>     return FALSE;
>   if ((next = next->next_bb) != join_bb && join_bb != EXIT_BLOCK_PTR)
>     {
>       if (else_bb)
>         join_bb = NULL;
>       else
>         return FALSE;
>     }
> 
> somehow tries to guard against join_bb == EXIT_BLOCK_PTR but fails.
> Thus, why not do that explicitely here instead of just in the
> single case you cover?  (I can't see why join_bb could not be
> set to EXIT_BLOCK_PTR in some weird case)

>From my reading of the code, it can handle the normal case where
join_bb is EXIT_BLOCK_PTR just fine, provided that single_succ (then_bb)
== join_bb and !else_bb || single_succ (else_bb) == join_bb.
The ICE is there only because of the extra optimization I've tweaked,
the problem is there that then_bb has no successors and join_bb is
EXIT_BLOCK_PTR, so while then_bb can be successfully merged together with
test_bb, it has no successor and as join_bb is EXIT_BLOCK_PTR, we just give
up.  If then_bb has no successor and join_bb isn't EXIT_BLOCK_PTR, we'd
normally do:
  else if (EDGE_COUNT (join_bb->preds) < 2
           && join_bb != EXIT_BLOCK_PTR)
    {
      /* We can merge the JOIN cleanly and update the dataflow try
         again on this pass.*/
      merge_blocks (combo_bb, join_bb);
      num_true_changes++;
    }
and all is fine, and if then_bb (and else_bb if it exists) has a single
successor of join_bb, all is fine too.

	Jakub


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]