This is the mail archive of the
mailing list for the GCC project.
Re: [PATCH] Fix cond_exec_find_if_block (PR rtl-optimization/56745)
- From: Jakub Jelinek <jakub at redhat dot com>
- To: Richard Biener <rguenther at suse dot de>
- Cc: Eric Botcazou <ebotcazou at libertysurf dot fr>, gcc-patches at gcc dot gnu dot org
- Date: Tue, 2 Apr 2013 15:31:15 +0200
- Subject: Re: [PATCH] Fix cond_exec_find_if_block (PR rtl-optimization/56745)
- References: <20130402114657 dot GK20616 at tucnak dot redhat dot com> <alpine dot LNX dot 2 dot 00 dot 1304021455070 dot 21094 at zhemvz dot fhfr dot qr>
- Reply-to: Jakub Jelinek <jakub at redhat dot com>
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
> first condition for free, since we've already asserted that there's a
> fallthru edge from IF to THEN. Likewise for the && and || blocks,
> we checked the FALLTHRU flag, those are already adjacent to the last
> 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;
> 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
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);
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.