[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 Tue, Jun 28, 2016 at 10:01 PM, Chandler Carruth <chandlerc at gmail.com>
wrote:

> chandlerc added inline comments.
>
> ================
> Comment at: lib/Analysis/CGSCCPassManager.cpp:327
> @@ -24,1 +326,2 @@
> +  return *C;
>  }
> ----------------
> >> As i somewhat alluded to above, what you describe should be handled by
> a ref edge turning into a call edge, and the update mechanism should be
> able to handle that well.
> >
> > Please add test cases exhibiting this.
>
> Er, the next sentence in what I wrote was:
>
> >> I've added a test case that exercises this with GVN and function-attrs.
>
> I think the updated patch has this test case in it. It is
> cgscc-observe-devirt.ll
>
> >> The direct answer to your question is #2: it continues running on the
> now-larger SCC, detects that we switch from one SCC to another at some
> point, and re-runs on that SCC to make sure that the refined graph is
> observed.
> >
> > Okay, please add a test case for that.
>
> This is the same thing, and covered by the same test case. I was just
> trying to make sure I directly answer your question as well...
>

Sorry. I think I may have been looking at the earlier version of the patch.


>
>
> > Also add a test case demonstrating that we don't go quadratic on a graph
> like http://reviews.llvm.org/F2110388 where function passes manage to
> devirtualize all the ref edges.
>
> I'm not really sure what you want here... In general it is very hard to
> have a test case in the regression test suite that demonstrates a lack of
> quadratic behavior -- it typically requires an unacceptably large test case
> even when the behavior is linear.
>
> There are also a bunch of things that might "go quadratic" in this case.
> There are FIXMEs in the code for some of these things that I would like to
> address, but probably don't belong conflated into this patch...
>
> Based on the example you post, I think I've figured out that you are
> trying to point out a case where we will run the SCC pass manager over the
> function E as many times as we successfully devirtualize edges somewhere in
> the SCC containing E in a way that brings a new node into that SCC. If I've
> understood this correctly, then I agree, and that's a nice find. I think it
> is unlikely to be a problem in practice, but it is definitely something we
> would need fixed to finish deploying this, probably with just a cap to
> limit things as a very large SCC formed in this way seems unlikely to be a
> practical concern to optimize heavily. Given that, I'm inclined to make a
> FIXME or note about this rather than trying to address it within this patch
> as that seems like it would bottleneck things.
>
> Did I understand correctly? Does that approach make sense?
>

I was thinking about it in slightly different terms (running O(n) times on
an SCC of size O(n)) but yes, essentially that is my concern. It will
essentially be the same thing as the "max-cg-scc-iterations" limit, right?

-- Sean Silva


>
>
> http://reviews.llvm.org/D21464
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160629/440ba003/attachment.html>