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

Chandler Carruth via llvm-commits llvm-commits at lists.llvm.org
Fri Aug 5 22:20:44 PDT 2016


chandlerc added inline comments.

================
Comment at: include/llvm/Analysis/CGSCCPassManager.h:528
@@ +527,3 @@
+      // Update the SCC if necessary.
+      C = UR.UpdatedC ? UR.UpdatedC : C;
+
----------------
I'm not sure what you mean...

I think there are already test cases for invalidation, but I can double check. But that shouldn't be in *this* patch, that should be in the big CG update patch. I haven't gone back to that one to update it substantially (this iteration utility was requested and I wanted to get it out for review) but I'm happy to add test cases there, or add more testing in subsequent patches for that stack as problems are discovered.

================
Comment at: include/llvm/Analysis/CGSCCPassManager.h:557
@@ +556,3 @@
+        // something.
+        if (!F->isIntrinsic()) {
+          Devirt = true;
----------------
silvas wrote:
> Can you just avoid keeping WeakVH's to intrinsics? The langref says you can't take their address. Therefore, a devirtualized call will never be to an intrinsic.
> http://llvm.org/docs/LangRef.html#intrinsic-functions
It's not that we could create a VH to a call to an instrinsic.

Above, we create a VH to an indirect call. That indirect call gets devirtualized into a direct call. At some point, some pass may turn that direct call into a call to an intrinsic, and if it does that this code needs to defensively prepare for it.

Sadly, there is no pass in tree that does this, but without this logic the pass would crash if anything ever conspired to trigger this.


https://reviews.llvm.org/D23114





More information about the llvm-commits mailing list