<br><br><div class="gmail_quote">On Wed, Sep 21, 2011 at 3:36 PM, Chandler Carruth <span dir="ltr"><<a href="mailto:chandlerc@google.com">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 class="gmail_quote"><div class="im">On Wed, Sep 21, 2011 at 3:25 PM, David Blaikie <span dir="ltr"><<a href="mailto:dblaikie@gmail.com" target="_blank">dblaikie@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">

<div>One other point on this - the issues you've raised already exist in both assert and llvm_unreachable code, making this change (assert(0) -> llvm_unreachable) somewhat orthogonal to them.<br></div></blockquote>

<div><br></div></div><div>Not at all. llvm_unreachable has a tremendously different impact in optimized builds than assert(0) does. The latter has zero impact, the control flow remains as expected.</div><div><br></div><div>
But llvm_unreachable tells the optimizer that this code path *cannot be taken*. That in turn constrains the value of the input. That in turn causes other optimizations to fire, and so on. This can cause essentially unbounded undefined behavior given an unexpected input, where as the assert will "merely" fall through.</div>

<div><br></div><div>I've specifically seen cases in the disassembler and assembler code where truly unexpected cases are assert(0)'ed, but then handled benignly.</div></div></blockquote><div><br></div><div>That's pretty much pure luck & potentially just hides the bug for longer in retail builds that might otherwise fail more catastrophically (or less, certainly - but I don't think it's strictly better to have release builds be agnostic to these paths)</div>
<div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;"><div class="gmail_quote"><div> I don't think we want to tell the optimizer that this *cannot* happen, but rather catch when it does happen in our test suite. The assert seems well suited to that.</div>

</div>
</blockquote></div><br><div>Is this true even in debug builds? My understanding was that llvm_unreachable was much like an assert in debug builds - otherwise the use of a message is rather useless since the compiler might not even have such code anymore.<br>
<br>If it is true, should we consider changing our llvm_unreachable usage to assert(0s? That's what I was getting at above. We're already treating the same case in two different ways (some times with assert, sometimes with llvm_unreachable) & it'd be nice to figure out the right way.</div>
<div><br></div><div>[on that note about optimizations - it'd be nice if our asserts could be used to implement the same kind of cascading optimizations that llvm_unreachable provides, wouldn't it?]</div><div><br></div>
<div>- David</div>