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.