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

Hal Finkel via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Oct 5 12:52:11 PDT 2017


hfinkel added a comment.

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

> In https://reviews.llvm.org/D38566#889031, @hfinkel wrote:
>
> > > This is a minimal patch to solve PR34603:
> > >  https://bugs.llvm.org/show_bug.cgi?id=34603
> >
> > Can you please summarize here what's going on and how you arrived at this solution? Last I recall, in PR34603, we were discussing select formation, and how it might not be desirable to form selects early. By "aggressive flattening" do you mean select formation? What you're doing here seems like more than just disabling select formation, but sinking? I also think that we might not want to sink early, at least some things (because we can't later prove it safe to re-hoist), but it's not immediately clear how all of these things are related.
>
>
> Sorry for the invented vocabulary. 'Aggressive flattening' was meant to include the select formation, but that's not actually the root of the problem. The sinking is the root of the problem, so that's why I'm proposing to disable that in the early incarnation of SimplifyCFG. My intent (probably needing refinement) is to delay the transform that was added with https://reviews.llvm.org/rL279460 ( SinkThenElseCodeToEnd ) because that's where we regressed the examples in the bug reports. Let me illustrate using the example in the test cases (it's a slightly simplified form of the motivating example in the bug report):


Thanks for the explanation. So, to be clear, we're only delaying the "sinking through PHIs" type of sinking until late?

I'm a bit concerned about the fact that LateSimplyCFG currently runs right after the SLP vectorizer, but we probably want to do select formation aggressively before the SLP vectorizer. We might, in fact, want to form selects more aggressively before the loop vectorizer too. In short, I'm not sure that this variety of "late" is the same as our existing concept of "late" (which is something that interferes with loop structure, and thus, should come after vectorization (at least loop vectorization)).


https://reviews.llvm.org/D38566





More information about the llvm-commits mailing list