<br><div class="gmail_extra"><div class="gmail_quote">On Wed, Jul 25, 2012 at 1:13 AM, Duncan Sands <span dir="ltr"><<a href="mailto:baldrick@free.fr" target="_blank" class="cremed">baldrick@free.fr</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Hi Petar,<br>
<div class="im"><br>
> as I mentioned in the previous email, the bug report I pasted link for was the original root cause. There seem to be more issues caused by __builtin_unreachable, and toolchains that have a fix for that one (for instance latest Google 4.6.x toolchain for Android) still have issues with __builtin_unreachable for MIPS.<br>

><br>
> We will inevitably resolve all these issues (we are debugging the latest occurrence now), but until that point is reached, we need to compile LLVM correctly.<br>
<br>
</div>if this is a question of "make llvm_unreachable more friendly while we fix the<br>
MIPS backend" why not just apply your change locally rather than in the main<br>
repository?</blockquote><div><br></div><div>I don't think that's a quite accurate portrayal of the issue... it's more "our host compilers are full of bugs w.r.t. __builtin_unreachable". I think that's an issue w/ the host compiler, and folks should either A) find a better host compiler, or B) bootstrap and use a Clang-built tree.</div>
<div><br></div><div>If B doesn't work for some reason, you should work on that. ;] Patches very welcome there.</div><div><br></div><div>I don't think its reasonable to hack around these broken compilers inside the LLVM codebase. Applying local patches to satisfy your build environment is a common reality.</div>
<div><br></div><div><br></div><div>I actually have a lot of sympathy for Reed's comment, but the fact is that this patch hacks around bugs in the host compiler. That is a very slippery slope, and frankly we are already sliding. I don't want more and more hacks to satisfy a host compiler. This is especially true if the host compiler with problems is some specific toolchain that none else would be using. You should file bugs and get that toolchain fixed, and/or you should come up with a bootstrap solution.</div>
<div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">That said, I personally think it was a mistake to replace<br>
assert(false) with llvm_unreachable in places that may be reachable but<br>
shouldn't happen.<br></blockquote><div><br></div><div>I think the current llvm_unreachable definition is very workable -- it has nice asserting logic in debug builds.</div><div><br></div></div></div>