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

David Blaikie via llvm-commits llvm-commits at lists.llvm.org
Mon Apr 4 13:18:39 PDT 2016


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*

(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
> >>
> >>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160404/dd27190c/attachment.html>


More information about the llvm-commits mailing list