[PATCH] D108741: [GlobalDCE] Handle non-vfunc entries in vtables during VFE

[Florian Hahn via Phabricator via llvm-commits]

fhahn accepted this revision.
fhahn added a comment.
This revision is now accepted and ready to land.

LGTM with a few remaining nits. Please wait a couple of days in case there are additional comments.



================
Comment at: llvm/lib/Transforms/IPO/GlobalDCE.cpp:140
+      if (auto *VTable = dyn_cast<GlobalVariable>(GVU)) {
+        IgnoreDependency = VFuncMap[VTable].count(&GV) > 0;
+      }
----------------
nit: `contains` instead of `count`


================
Comment at: llvm/lib/Transforms/IPO/GlobalDCE.cpp:141
+        IgnoreDependency = VFuncMap[VTable].count(&GV) > 0;
+      }
+    }
----------------
coding standard recommends not using `{}` for single line statements https://llvm.org/docs/CodingStandards.html#don-t-use-braces-on-simple-single-statement-bodies-of-if-else-loop-statements


================
Comment at: llvm/lib/Transforms/IPO/GlobalDCE.cpp:190
     // the list of (GV, offset) pairs which are the possible vtables for that
     // typeid.
     for (MDNode *Type : Types) {
----------------
nit: .... also collect the set of virtual functions  in the vtable.


================
Comment at: llvm/lib/Transforms/IPO/GlobalDCE.cpp:205
+          VFuncs.insert(F);
+        }
+      }
----------------
coding standard recommends not using {} for single line statements https://llvm.org/docs/CodingStandards.html#don-t-use-braces-on-simple-single-statement-bodies-of-if-else-loop-statements


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D108741/new/

https://reviews.llvm.org/D108741