On Wed, 24 Oct 2012, Steven Bosscher wrote:

> On Wed, Oct 24, 2012 at 4:25 PM, Richard Biener <rguent...@suse.de> wrote:
> > Comments?  Should find_many_sub_basic_blocks really do what
> > it does here?  Or is the CFG in fact "invalid" (but we don't
> > check that in verification)?
> 
> If a "noreturn" function is inlined, doesn't the inliner detect that
> the inlined body does in fact return?? The error is that the function
> "bar()" returns, the documentation for noreturn makes it pretty clear
> that such functions should never return:
> 
> In this case, bar() returns but the compiler has already used its
> noreturn attribute to construct a dead end in the CFG (i.e. your basic
> block without successors). But the fact that bar() returns is
> undefined behavior. I'm not sure it is possible to know where the
> function would continue after returning from bar(), after lowering to
> GIMPLE.
> 
> IMHO find_many_sub_basic_blocks should not connect this block to the
> rest of the CFG, so expanding a trap or a barrier is my preferred
> solution, like so:
> 
> Index: cfgexpand.c
> ===================================================================
> --- cfgexpand.c (revision 192696)
> +++ cfgexpand.c (working copy)
> @@ -3989,6 +3989,12 @@ expand_gimple_basic_block (basic_block b
> 
>    do_pending_stack_adjust ();
> 
> +  /* If this block has no successors at all, make sure that
> +     find_many_sub_basic_blocks does not connect this "dead end"
> +     to the rest of the function.  */
> +  if (EDGE_COUNT (bb->succs) == 0)
> +    expand_builtin_unreachable ();
> +
>    /* Find the block tail.  The last insn in the block is the insn
>       before a barrier and/or table jump insn.  */
>    last = get_last_insn ();
> 
> 
> At least this way the GIMPLE and RTL CFGs should be more alike than
> with an infinite loop.

Sure.  My patch made sure we never have a BB without successor
like this on GIMPLE either.  I suppose DOM computation wouldn't
like it (not sure if noreturn/infinite-loop to exit connect does
the right thing here).

Other places to fix it up like you do (with __builtin_uneachable ())
are in lowering a noreturn function (place a unreachable () instead of
a return stmt in the return block), the inliner (place a unreachable ()
on the exit edge) and fixup_cfg (where I put the endless loop in).

> But, really, why inline noreturn functions at all?

That's a good question.  The question then is, why not?

And the question still stands: is a BB without successor ok?

Thanks,
Richard.

Reply via email to