[PATCH] D21464: [PM] WIP: Introduce basic update capabilities to the new PM's CGSCC pass manager, including both plumbing and logic to handle function pass updates.

Chandler Carruth via llvm-commits llvm-commits at lists.llvm.org
Tue Jun 28 17:55:28 PDT 2016


chandlerc added a comment.

In http://reviews.llvm.org/D21464#466098, @davidxl wrote:

> Chandler, thanks for the tremendous effort for trying to make this work. At this moment, I only have very high level comments,  so please bear with me.
>
> Can we have a design document describing the underlying algorithm with formal proof (traversing with mutation)? Given the level of complexity involved and the fact there is no prior literature we can refer to, it is important we get it right.


I've added an overview to the top of the file that I hope answers a lot of the high level questions.

I don't have a formal proof of this, nor do I really know what you would want to prove here. What's the concern? I am working on building up a set of test cases that I think exercise the kinds of updates. I've not finished with that yet, but so far it seems to be hitting lots of the different aspects of this code.

> Besides  it is also  more important to understand what problem the added complexity is trying to solve, why those problems are important to be solved (evidence of missing opportunities in real apps), and why they can not be solved in other ways?


See my response to Sean and my comments in the updated patch (coming shortly) for some of the details here.

I view this much more about getting the call graph management into a principled state with a path that supports the different goals. The new pass manager's requirement of *identity* for things to support analysis caching was a key element that drove the development of the LCG in the beginning. Without that kind of identity, it becomes extremely hard to reason about a caching approach to analyses. I can't claim its impossible to get something to work, but I tried and was unable to see a clear and clean path that made sense to me, so I worked on building up the infrastructure necessary to have identity.

Once you have identity, you need to be very careful with updates because it makes it easy to transform a "benign" relaxation of the model (the old PM's approach of folding more nodes into the current SCC) into a correctness bug. As a consequence I've tried to take a very principled approach to the call graph update where we can verify and check that the graph structure has a particular shape and structure.

Ultimately, I think this is the right way to design the call graph portion of the pass manager because I think other designs will inevitably run up against the limitations that their relaxed model imposes. In fact, I'd like to generalize and make more principled even the idea of the "reference graph" because I think it hasn't gone far enough yet, but that's for a future iteration.

> The same questions have been raised in Sean's email thread, and there are some interesting discussions there, so may be you can also chime in there?


That email thread happened when I didn't have time to read it as it went, and after skimming it I was unable to really understand what open questions there were.

This *has* actually been discussed before, if you look at the discussions that led to the LazyCallGraph design. I don't know that we have a complete record of the discussion, but I've tried to capture the sentiment that folks were left with in terms of what direction the design should go in the comment I added to the top of the file.

None of that is to say that we shouldn't revisit the design decisions if they are wrong of course! I just don't want to give the impression that there was *no* discussion about this. We should still try to figure out if there is a problem with this approach or a better approach that we should take.

> As far as I can tell, for the new PM transition,  the consensus is to make the transition NFC initially, which will greatly lower the bar for the new PM to become the default.  If your plan is to make new PM transition depending on this update support in CGSCC pass to be ready,  we need to understand why.


I don't actually agree with this. This has come up several times on several different threads with myself, Hal, Philip and numerous others.

I think it would be *nice* if we could make the transition NFC, but I personally do not believe that is reasonable given the degree of difference between the designs. To me, it is very fundamental that when you have a caching analysis layer that understands the SCC organizational unit of IR, you need a different strategy for handling updates and iteration. Trying to force things to look exactly the same will, IMO, cause the design of the new pass manager to be worse.

That said, we do have to have a way to migrate. My plan has been to simply make the new approach superior in terms of benchmark numbers, and at least not a significant regression in terms of compile time. Given the power of a more call graph aware refinement-driven iteration order, and the compile time savings of caching analysis, I think both of these will be reasonably attainable. However, if we try, and they prove very difficult, I think we will still be able to layer a constraint on top of the current design that more precisely mimics the old behavior in order to ease migration. I don't think it will come to this, and I also think that if it does, having what we think is the principled design (but which needs more work to address regressions) underneath will ensure that the compatibility mode sits cleanly on top and doesn't twist the core in an unfortunate direction.


http://reviews.llvm.org/D21464





More information about the llvm-commits mailing list