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

[Sean Silva via llvm-commits]

On Thu, Jun 30, 2016 at 4:26 PM, Xinliang David Li <davidxl at google.com>
wrote:

>
>> I would still really love to talk about any concerns or problems you see
>> with the design. Currently the one I'm thinking about the most is quadratic
>> behaviors, but if there are other concerns I'd like to discuss them.
>>
>
>
> I may be repeating myself, but I would like to summarize my concerns and
> position here:
>
> 1. We have a goal to make the pass manager to use the new PM. There are
> many known/concrete performance improvements depending on that (there is
> possible short term solution that does not depend on new PM, but we've
> decided to make them depend on new PM switch anyway).  Note the PM switch
> is already a very large task with potential risks, coupling that task with
> another huge change is not the right plan going forward.  The right way is
> to make *minimal* changes enable new PM as soon as possible and make
> incremental improvement of other parts later.  The longer we delay, the
> longer the community suffers.
>
> 2. Changes with this scale needs to be justified with concrete benefit. If
> there are anything real missing, we should have known them already.
>
> 3. With changes in core infrastructure like this, subtle bugs are likely
> to be very hard to debug. When a bug is introduced, there will no way for
> user to workaround it (such as by disabling a pass). This is reason why
> core piece like this needs to kept simple
>
> 4. The patch introduces a new SCC formation algorithm (double layer with
> support for CG mutation on the fly), however design document on how this
> works is missing.  It needs to document
>     * What exactly is expected (in terms of vistation order) when a) an
> edge is added; b) an edge is deleted; c) when a new node is introduced and
> connected.
>     * How ref-edges are formed, how indirect callsites are handled etc.
>     * If it is a modification of the classic SCC formation algorithm,
> describe the change and prove that the algorithm works as expected.
>
> Having a document like this also helps reviewers of the code (for
> reasoning of the correctness).  Without this, I suspect very few people can
> help with maintaining the code in the future.
>
> 5.  Another added complexity is that the SCC passes are now required to do
> the updates -- it seems pretty obscure to the pass writers.
>
> 6. What is the plan to address the quadratic behavior?
>
> 7. Is this tested on large programs (e.g, in LTO mode)?  How large is the
> RefSCC graph? How much memory overhead is expected? What is the compile
> time impact?
>
> 8. This change makes new PM transition not NFC.  It can potentially affect
> a lot of other people, e.g., introduced performance regression, correctness
> issue etc.
>
> So in short, my proposal is that we do minimal work in CGSCC pass manager
> to enable/unblock porting of inliner to the the new PM, and we can
> revisit/re-examine this patch in depth later when new PM is in place.  Does
> it make sense?
>

I have posted the proof of concept port of the old PM visitation to the new
PM. http://reviews.llvm.org/D21921
The proof of concept also ports all the remaining CGSCC passes as a quick
sanity check.

Please take a look, everybody.

-- Sean Silva



>
> thanks,
>
> David
>
>
>>
>>
>>>
>>>
>>>>
>>>>
>>>>> Regardless, if/when the evidence appears for those cases being
>>>>> important to optimize, we can add them; porting to the new PM is orthogonal.
>>>>>
>>>>> [*] Hopefully I will start an RFC and send out a proof of concept
>>>>> patch tomorrow; I just need to test out a proof of concept port of the
>>>>> inliner on something bigger to make sure everything works.
>>>>>
>>>>
>>>> As I have said in this thread, several other threads when LazyCallGraph
>>>> was first designed, and in several other discussions, I continue to think
>>>> this is the wrong approach.
>>>>
>>>
>>> Can you provide links to those discussions? I can't find anything in my
>>> email.
>>>
>>
>> I have tried very hard to find them, and haven't succeeded. Really sorry
>> about this. I know some of them took place outside of email sadly, but my
>> memory is that some did take place in the discussion threads around the
>> pass manager and the need for having a cache key for SCCs.
>>
>> That said, my only real goal in mentioning this is that I don't want you
>> or others to think that this is something that was never discussed. I even
>> clearly remember Hal having reservations about this. ;] I know I myself
>> would have preferred to maintain precise similarity to the current pass
>> manager, but I don't see a way that results in a robust interaction with
>> cache based management of analyses.
>>
>> Anyways, I'm happy to try to explain this freshly if useful, but I'm not
>> sure what the remaining points of contention or confusion are.
>>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160630/c87d5813/attachment.html>