<div dir="ltr">Totally up to you whether you think it's worth it. I don't know enough about the code or what the bad things are that might happen if we miss this code when it would fire... *shrug*<br><br>(might be able to simplify the assert a bit with llvm::* versions of all_of and find that take ranegs rather than begin/end (not sure if we have those specific algorithms in range versions, but easy to add if we don't))</div><div class="gmail_extra"><br><div class="gmail_quote">On Mon, Apr 4, 2016 at 11:59 AM, Justin Bogner <span dir="ltr"><<a href="mailto:mail@justinbogner.com" target="_blank">mail@justinbogner.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">David Blaikie <<a href="mailto:dblaikie@gmail.com">dblaikie@gmail.com</a>> writes:<br>
> If you want to be defensive, youc ould turn the whole loop into an<br>
> assert(any_of(...)) - that way we'll find it if the situation comes up for<br>
> anyone doing a +Asserts build, and we can get a test case and from<br>
> there/decide if we need the functionality, etc.<br>
<br>
</span>The assert ends up being a bit intense. Patch attached. Do you think<br>
it's worth it? If not I'll just do the removal by itself.<br>
<br>
<br><br>
<br>
> On Thu, Mar 31, 2016 at 11:31 AM, Justin Bogner via llvm-commits <<br>
> <a href="mailto:llvm-commits@lists.llvm.org">llvm-commits@lists.llvm.org</a>> wrote:<br>
><br>
>> There's some code near the end of TailDuplicate to handle "the nasty<br>
>> case in that we duplicated a block that is part of a loop into some but<br>
>> not all of its predecessors."<br>
>><br>
>> I noticed that this isn't covered by our existing tests and spent some<br>
>> time trying to come up with an example it actually hits. I tried hand<br>
>> rolling something based on the explanation in the comment, but couldn't<br>
>> get anything that didn't abort tail duplication earlier for one reason<br>
>> or another.<br>
>><br>
>> Then, I applied the attached patch (with tail-dup-size cranked up so<br>
>> this would fire more) and ran a bootstrap of clang and the nightly test<br>
>> suite - the assert was never hit.<br>
>><br>
>> I suspect that things have changed and the situation this code handles<br>
>> can't happen anymore. Should we remove it?<br>
>><br>
>><br>
>> _______________________________________________<br>
>> llvm-commits mailing list<br>
>> <a href="mailto:llvm-commits@lists.llvm.org">llvm-commits@lists.llvm.org</a><br>
>> <a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits</a><br>
>><br>
>><br>
<br></blockquote></div><br></div>