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

[Peter Collingbourne via Phabricator via llvm-commits]

pcc added inline comments.


================
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)) {
----------------
tejohnson wrote:
> 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?
If we would have early returned before, we will need to run this check in order to try to export any type IDs. There is no code to handle exporting right now, but it will be added separately later (see D29811).


================
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
----------------
tejohnson wrote:
> How do we know which were not devirtualized? We fall through to here even after the above code which does devirtualizaton.
The idea is that if devirtualization was successful we will clear the TypeCheckedLoadUsers vector. I have added a comment explaining this.


https://reviews.llvm.org/D29808