[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]

On Thu, Jul 7, 2016 at 2:30 PM Sean Silva via llvm-commits <
llvm-commits at lists.llvm.org> wrote:

> On Tue, Jun 28, 2016 at 5:55 PM, Chandler Carruth <chandlerc at gmail.com>
> wrote:
>
>> 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.
>>
>
> Chandler, can you please point to where in the code where "the old PM's
> approach of folding more nodes into the current SCC" happens? I'm pretty
> sure it does not happen, because doing that is precisely what one of the
> changes in http://reviews.llvm.org/P6012 (quick hack) does, and I had to
> make the `Nodes` member of CallGraphSCC public in order to do it (no
> included in that patch for brevity).
>

Sorry, I was remembering the ReplaceNode functionality that
ArgumentPromotion uses to mutate the SCC. You're correct that we don't add
nodes to the SCC.

In summary, we neither remove nodes from an SCC when cycles are broken nor
do we add nodes to the SCC when we introduce a cycle. The latter is
conservatively correct (I think) because the SCC was modeled as having an
indirect call which is more conservative than if we merged a concrete node
into the SCC and treated it as a direct call.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160707/98ad51a8/attachment.html>