The logic at the end of TailDuplicate seems dead - can we remove it?

Justin Bogner via llvm-commits llvm-commits at lists.llvm.org
Mon Apr 4 14:18:15 PDT 2016


David Blaikie <dblaikie at gmail.com> writes:
> 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*

Committed in r265347, with the assert.

> (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))
>
> On Mon, Apr 4, 2016 at 11:59 AM, Justin Bogner <mail at justinbogner.com>
> wrote:
>
>> David Blaikie <dblaikie at gmail.com> writes:
>> > If you want to be defensive, youc ould turn the whole loop into an
>> > assert(any_of(...)) - that way we'll find it if the situation comes up
>> for
>> > anyone doing a +Asserts build, and we can get a test case and from
>> > there/decide if we need the functionality, etc.
>>
>> The assert ends up being a bit intense. Patch attached. Do you think
>> it's worth it? If not I'll just do the removal by itself.
>>
>>
>>
>>
>> > On Thu, Mar 31, 2016 at 11:31 AM, Justin Bogner via llvm-commits <
>> > llvm-commits at lists.llvm.org> wrote:
>> >
>> >> There's some code near the end of TailDuplicate to handle "the nasty
>> >> case in that we duplicated a block that is part of a loop into some but
>> >> not all of its predecessors."
>> >>
>> >> I noticed that this isn't covered by our existing tests and spent some
>> >> time trying to come up with an example it actually hits. I tried hand
>> >> rolling something based on the explanation in the comment, but couldn't
>> >> get anything that didn't abort tail duplication earlier for one reason
>> >> or another.
>> >>
>> >> Then, I applied the attached patch (with tail-dup-size cranked up so
>> >> this would fire more) and ran a bootstrap of clang and the nightly test
>> >> suite - the assert was never hit.
>> >>
>> >> I suspect that things have changed and the situation this code handles
>> >> can't happen anymore. Should we remove it?
>> >>
>> >>
>> >> _______________________________________________
>> >> llvm-commits mailing list
>> >> llvm-commits at lists.llvm.org
>> >> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>> >>
>> >>
>>
>>


More information about the llvm-commits mailing list