[PATCH] D29808: WholeProgramDevirt: Add any unsuccessful llvm.type.checked.load devirtualizations to the list of llvm.type.test users.

[Teresa Johnson via Phabricator via llvm-commits]

tejohnson added inline comments.


================
Comment at: llvm/lib/Transforms/IPO/WholeProgramDevirt.cpp:960
 
-  if ((!TypeTestFunc || TypeTestFunc->use_empty() || !AssumeFunc ||
+  if (Action != PassSummaryAction::Export &&
+      (!TypeTestFunc || TypeTestFunc->use_empty() || !AssumeFunc ||
----------------
I guess the early return here was for the case when there could not be any devirtualization - suggest adding a comment that in the Export case we still need to go through and handle the non-devirtualized llvm.type.checked.load intrinsics.


================
Comment at: llvm/lib/Transforms/IPO/WholeProgramDevirt.cpp:1015
     std::vector<VirtualCallTarget> TargetsForSlot;
-    if (!tryFindVirtualCallTargets(TargetsForSlot, TypeIdMap[S.first.TypeID],
-                                   S.first.ByteOffset))
-      continue;
-
-    if (!trySingleImplDevirt(TargetsForSlot, S.second) &&
-        tryVirtualConstProp(TargetsForSlot, S.second))
+    if (tryFindVirtualCallTargets(TargetsForSlot, TypeIdMap[S.first.TypeID],
+                                  S.first.ByteOffset)) {
----------------
Is it worth skipping this check in the case where we previously had an early return above, but now will come through here in the Export case?


================
Comment at: llvm/lib/Transforms/IPO/WholeProgramDevirt.cpp:1029
+    // CFI-specific: if we are exporting and any llvm.type.checked.load
+    // intrinsics were *not* devirtualized, we need to add the resulting
+    // llvm.type.test intrinsics to the function summaries so that the
----------------
How do we know which were not devirtualized? We fall through to here even after the above code which does devirtualizaton.


https://reviews.llvm.org/D29808