<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On 21 September 2016 at 10:11, Hans Wennborg <span dir="ltr"><<a href="mailto:hans@chromium.org" target="_blank">hans@chromium.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">hans added a comment.<br>
<span class=""><br>
In <a href="https://reviews.llvm.org/D24680#545412" rel="noreferrer" target="_blank">https://reviews.llvm.org/<wbr>D24680#545412</a>, @nlewycky wrote:<br>
<br>
> I'm concerned that the fix is too narrow. There's 10 callers to llvm::SplitEdge in LLVM and haven't audited them so I don't know whether they also check whether they also do this check. It's certainly not in the comment on SplitEdge that it's allowed to assert if you pass it a landing pad block.<br>
<br>
<br>
</span>My thinking was that it seems what jump threading is trying to do with a "branch on xor" pattern isn't going to work with a landingpad anyway, and this would fix the crash that people run into in practice.<br>
<br>
I agree it doesn't fix the wider problem of what to do when SplitEdge fails, but it seems like a good improvement to me.<br></blockquote><div><br></div><div>I'm not opposed to landing this patch, it's an obviously correct fix to a bug with multiple dupes.</div><div><br></div><div>I do think the PR isn't really fixed until the underlying API gets fixed, and would like the PR would stay open to track the design problem even if it's asymptomatic once this patch lands.</div><div><br></div><div>Nick</div><div><br></div></div></div></div>