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

[Kuba (Brecka) Mracek via Phabricator via llvm-commits]

kubamracek added inline comments.


================
Comment at: llvm/docs/TypeMetadata.rst:288
+
+1. All virtual function pointers in the vtable must have a matching ``!type``
+attachment. Function pointers without a ``!type`` attachment are not
----------------
fhahn wrote:
> Not sure about the rst syntax here. Does this render correctly as list?
Fixed!


================
Comment at: llvm/lib/Transforms/IPO/GlobalDCE.cpp:149
     }
+
     GVDependencies[GVU].insert(&GV);
----------------
fhahn wrote:
> nit: unrelated change
Removed.


================
Comment at: llvm/lib/Transforms/IPO/GlobalDCE.cpp:184
     Types.clear();
+    VFuncs.clear();
     GV.getMetadata(LLVMContext::MD_type, Types);
----------------
fhahn wrote:
> can we instead just declare VFuncs in the loop?
Done.


================
Comment at: llvm/lib/Transforms/IPO/GlobalDCE.cpp:204
+                                           *GV.getParent(), &GV);
+      C = C ? C->stripPointerCasts() : nullptr;
+      if (auto VFunc = dyn_cast_or_null<GlobalValue>(C)) {
----------------
fhahn wrote:
> Does C being `nullptr` here indicate that the type id is invalid?
Yes, it means the offset mentioned in the !type attachment isn't found in the vtable. I restructured this to avoid the ternary operator.


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

https://reviews.llvm.org/D108741