[PATCH] D21921: [proof of concept] Port old PM CGSCC visitation logic to new PM

[Davide Italiano via llvm-commits]

On Jul 3, 2016 11:22 PM, "Sean Silva via llvm-commits" <
llvm-commits at lists.llvm.org> wrote:
>
> silvas added a comment.
>
> Thanks for the comments Davide. As a quick reminder, the purpose of this
patch is just to be a proof of concept that we can all agree is the right
direction (or figure out why not). The patch is not intended to be
committed as-is to trunk, so I'd like to avoid discussing usual code
hygiene / "janitorial" details -- I've deliberately skimped on some of them
or done things in a suboptimal way just to keep things mechanical.
>
>
> ================
> Comment at: include/llvm/Analysis/CGSCCPassManager.h:104
> @@ +103,3 @@
> +
> +    // XXX: no existing CGSCC passes use this.
> +    //bool Changed = doInitialization(CG);
> ----------------
> davide wrote:
> > just remove it ? =)
> I guess I should have said this before, but all the things marked with
XXX are things that I wanted to highlight for reviewers. They are just
comments about non-obvious things that I thought reviewers would find
useful. For example, if somebody is looking at the code of
`CGPassManager::runOnModule` and comparing it with here, this will save
them the time of going and looking through the existing passes just to come
to the same conclusion.
>
> Some of the XXX comments will likely turn into regular comments, other
will just be deleted because they are only relevant to this proof of
concept and won't be issues for the real patches that will be committed to
trunk (e.g. commented-out code :) ).
>

Fair enough. If it wasn't clear, +1 from me on this as general direction. I
agree we can settle details later.

> ================
> Comment at: lib/Transforms/IPO/FunctionAttrs.cpp:1027
> @@ -1081,2 +1026,3 @@
>      Function *F = I->getFunction();
> -    if (!F || F->hasFnAttribute(Attribute::OptimizeNone)) {
> +    // XXX: The isDeclaration check here is because in the new PM BasicAA
> +    // uses DomTree which will cause it to crash on declarations.
> ----------------
> davide wrote:
> > XXX -> FIXME? Here and everywhere else in the patch where it applies
> It isn't a FIXME. It is a note for reviewers. It will turn into just a
regular comment since there isn't really anything to "fix" per se.
>
>
> http://reviews.llvm.org/D21921
>
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160703/80c4dfee/attachment.html>