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

[Sean Silva via llvm-commits]

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 :) ).

================
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