[PATCH] D23114: [PM] Introduce a devirtualization iteration layer for the new PM.

[Sean Silva via llvm-commits]

silvas added inline comments.

================
Comment at: include/llvm/Analysis/CGSCCPassManager.h:528
@@ +527,3 @@
+      // Update the SCC if necessary.
+      C = UR.UpdatedC ? UR.UpdatedC : C;
+
----------------
chandlerc wrote:
> silvas wrote:
> > If you update `C` in this assignment, where do you call `AM.invalidate(...)` on the original `C`? (this is needed to downward invalidate any function analyses that might depend on SCC analyses on the original `C`). Is this something you expect the SCC transformation being run by DevirtIteratingWrapper itself to do? It would be nice to see a unittest exhibiting this.
> The pass is required to preserve the call graph (that's part of passing it by argument) and if it updates it to reflect that in the UpdateResult and invalidate the analysis manager.
> 
> But all of this should really be spelled out in the prior patch that switches to this API, I'll update that patch with better documentation of this fact.
> 
> I certainly want to add verification of these properties as well, but I'd like to do that in a follow-up patch rather than put more into the existing patches out for review. (There is some verification already, but with some API changes and work I think we can get more verification into place.)
> But all of this should really be spelled out in the prior patch that switches to this API, I'll update that patch with better documentation of this fact.

Please add a test case as well.


https://reviews.llvm.org/D23114