[PATCH] D84999: dynamic_cast optimization in LTO

[Teresa Johnson via Phabricator via llvm-commits]

tejohnson added a comment.

I'm ok with regular LTO implementation going in first, but it would be great to have a ThinLTO follow on implementation. Happy to provide pointers on that when you are ready.

I have a couple of comments below after a very quick skim. One is a bigger concern (the mangling prefetch matching in llvm). Hopefully others will provide some input on that as well.



================
Comment at: clang/lib/CodeGen/ItaniumCXXABI.cpp:1474
 
+static void ComputeExtraHint(llvm::CallInst *Call, llvm::Value *HintArg,
+                             const CXXRecordDecl *Src, const CXXRecordDecl *Dst)
----------------
Needs a description as to what this function is doing. Also a much more specific name. "ExtraHint" doesn't say much.


================
Comment at: llvm/lib/Transforms/IPO/DynamicCastOpt.cpp:131
+// common strings
+StringRef TIPrefix("_ZTI");     // _ZTI1B = typeinfo for B
+StringRef VTablePrefix("_ZTV"); // _ZTV1B = vtable for B
----------------
Not a language expert, but I don't think it is safe or a good idea for LLVM to be making assumptions about the mangling (which can vary e.g. Microsoft is quite different). You probably need to have clang encode additional info in either metadata or an llvm intrinsic. Perhaps you can leverage the type checks normally inserted for vtable loads for WPD under -fwhole-program-vtables.


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

https://reviews.llvm.org/D84999