[PATCH] D38566: [SimplifyCFG] don't sink common insts too soon (PR34603)

Davide Italiano via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Nov 15 12:21:51 PST 2017


davide added a comment.

In https://reviews.llvm.org/D38566#926472, @spatel wrote:

> In https://reviews.llvm.org/D38566#926465, @davide wrote:
>
> > Until ~ 1 year ago, SimplifyCFG was a single pass, and everything was fine (and IMHO, that's how it should be).
> >  Then we found out it had bad interactions with inlining, so we decided to split in two passes. That's OK, as long as it's una tantum.
> >  Keeping adding knobs to tweak the 4-5 different SimplifyCFG invocations in-tree is a slippery rope.
>
>
> I'm not opposed to splitting it up, but that's a larger effort / follow-up to this.
>  Creating a struct of options for this pass was discussed as a reasonable way to solve PR34603, so that's what I've implemented:
>  https://bugs.llvm.org/show_bug.cgi?id=34603#c20


Once you end up adding n knobs people start relying on them and splitting the pass becomes more complicated, so "doing that as a follow-up" doesn't really work unless you have a clear plan in mind on how to split. If the majority thinks this is OK, I'm not going to argue further, but this combinatorial explosion of options introduced is clearly a symptom of a larger problem,  IMHO.


https://reviews.llvm.org/D38566





More information about the llvm-commits mailing list