<div dir="ltr">On Wed, Jan 23, 2013 at 10:27 AM, Daniel Dunbar <span dir="ltr"><<a href="mailto:daniel@zuster.org" target="_blank">daniel@zuster.org</a>></span> wrote:<br><div class="gmail_extra"><div class="gmail_quote">
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><br><div class="gmail_extra"><br><br><div class="gmail_quote"><div class="im">On Tue, Jan 22, 2013 at 5:55 PM, Chandler Carruth <span dir="ltr"><<a href="mailto:chandlerc@google.com" target="_blank">chandlerc@google.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div>On Tue, Jan 22, 2013 at 5:42 PM, Argyrios Kyrtzidis <span dir="ltr"><<a href="mailto:kyrtzidis@apple.com" target="_blank">kyrtzidis@apple.com</a>></span> wrote:<br>
</div><div class="gmail_extra"><div class="gmail_quote"><div>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word"><div><div><div>On Jan 22, 2013, at 5:23 PM, Chandler Carruth <<a href="mailto:chandlerc@google.com" target="_blank">chandlerc@google.com</a>> wrote:</div>
<br><blockquote type="cite"><div dir="ltr">On Tue, Jan 22, 2013 at 4:15 PM, Daniel Dunbar <span dir="ltr"><<a href="mailto:daniel@zuster.org" target="_blank">daniel@zuster.org</a>></span> wrote:<br><div class="gmail_extra">
<div class="gmail_quote">
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div dir="ltr">Hi Richard,<div><br></div><div>I object pretty strongly to using unreachable here for C++ code bases.</div>
</div></blockquote><div><br></div><div>There was some discussion surrounding this, and I'm still pretty strongly in favor of it...</div></div></div></div></blockquote><div><br></div></div><div>IMHO you still haven't given enough justification; your proposed optimization opportunity inĀ <a href="http://lists.cs.uiuc.edu/pipermail/cfe-dev/2012-October/025326.html" target="_blank">http://lists.cs.uiuc.edu/pipermail/cfe-dev/2012-October/025326.html</a> is specific to a full covered switch, which can be handled with an optimization that targets this case.</div>
</div></div></blockquote><div><br></div></div><div>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.</div>
</div></div></div></blockquote><div><br></div></div><div>The right way to handle this is with appropriate annotations. This is simply not a good argument for emitting unreachable.</div><div class="im"><div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div>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.</div>
</div></div></div></blockquote><div><br></div></div><div>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".</div>
</div></div></div></blockquote><div><br></div><div style>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.</div>
<div style><br></div><div style>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.</div>
<div style><br></div><div style>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.</div>
<div style><br></div><div style><br></div><div style>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.</div>
<div style><br></div><div style>To sum up:</div><div style>- if we aren't going to define the behavior, we should trap. This will make both programs and future optimizations much more predictable.<br></div><div style>
- if we are going to trap, then when optimizing we should leverage that in the hope of secondary and tertiary simplifications</div></div></div></div>