On Wed, Jan 23, 2013 at 10:44 AM, Chandler Carruth <[email protected]>wrote:
> On Wed, Jan 23, 2013 at 10:27 AM, Daniel Dunbar <[email protected]> wrote: > >> >> >> >> On Tue, Jan 22, 2013 at 5:55 PM, Chandler Carruth >> <[email protected]>wrote: >> >>> On Tue, Jan 22, 2013 at 5:42 PM, Argyrios Kyrtzidis <[email protected] >>> > wrote: >>> >>>> On Jan 22, 2013, at 5:23 PM, Chandler Carruth <[email protected]> >>>> wrote: >>>> >>>> On Tue, Jan 22, 2013 at 4:15 PM, Daniel Dunbar <[email protected]>wrote: >>>> >>>>> Hi Richard, >>>>> >>>>> I object pretty strongly to using unreachable here for C++ code bases. >>>>> >>>> >>>> There was some discussion surrounding this, and I'm still pretty >>>> strongly in favor of it... >>>> >>>> >>>> IMHO you still haven't given enough justification; your proposed >>>> optimization opportunity in >>>> http://lists.cs.uiuc.edu/pipermail/cfe-dev/2012-October/025326.html is >>>> specific to a full covered switch, which can be handled with an >>>> optimization that targets this case. >>>> >>> >>> It is applicable to any circumstance where there is a programmer-known >>> invariant that all paths terminate in a return which is not statically >>> determinable. I've seen both predicated code with if/else chains of this >>> form, and switches. >>> >> >> The right way to handle this is with appropriate annotations. This is >> simply not a good argument for emitting unreachable. >> >> But I actually think we're approaching this from the wrong direction. The >>> C++ standard is extremely clear that this is UB. Given that, I think we >>> should aggressively tell users about this problem with their code. If the >>> optimization benefits aren't worthwhile, then I would argue for emitting >>> llvm.trap in *all* cases rather than just in non-optimized builds. While I >>> think the optimization benefits are both easy to get and worth it, I don't >>> have a collection of benchmarks that rely on it so I'm not going to fight >>> tooth and nail for it. I just don't understand sacrificing it when we don't >>> have to. >>> >> >> I'd much prefer llvm.trap in all cases instead of unreachable, although I >> still don't see much value in forcing users to fix code that from their >> perspective "works". >> > > The fundamental question is: do you want to try to define as a language > extension the behavior here? If so, we have a process for that, but I think > it will be a painful and fruitless one as I think this language behavior > was very well considered by the C++ committee. I do not think this behavior > was chosen in ignorance but in a desire that the language work in a certain > way. > > If we don't want to define the behavior here, we do a disservice to the > users and to our selves to allow it to persist in the code. Eventually it > *will* be exploited to the users detriment and we should tell the user > about it quickly and effectively. This is the argument for using llvm.trap. > This is the argument I feel *very* strongly about. > > The argument to switch from llvm.trap to unreachable in an optimized build > is simply one of principles: the user told us to optimize, and the language > gave us an opportunity. There isn't some practical, useful behavior being > blocked here. We should optimize these cases just as we optimize signed > integer overflow even if in the particular situation it didn't help the > performance or code size signficantly. > > > If you want to think about the extent to which this matters, consider > inlining. Once inlined, the undefined nature of this pattern is more likely > to result in an active bug, and thus the user will more greatly appreciate > the trap diagnosing it for them. Also, once inlined the unreachable has > much more power and can result in significant simplifications. > > To sum up: > - if we aren't going to define the behavior, we should trap. This will > make both programs and future optimizations much more predictable. > - if we are going to trap, then when optimizing we should leverage that in > the hope of secondary and tertiary simplifications > One more point: I'd be really happy to have -fsanitize=undefined-return or some such which can be *combined* with optimizations to help track down buggy code.
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
