[PATCH] D84999: dynamic_cast optimization in LTO

[wael yehia via Phabricator via llvm-commits]

w2yehia added inline comments.


================
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
----------------
w2yehia wrote:
> tejohnson wrote:
> > 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.
> a very valid cocern;
> I'm doing some refactoring to hide the ABIism behind an interface in this pass, and have that interface support Itanium ABI for now.
> Will update this patch shortly.
Hi Teresa (@tejohnson). I updated the patch as follows. 
1. create a utility class to represent C++ ABI related properties and queries (see `CXXABI.h`). 
2. create a `GuessABI` function that uses the triple to guess the C++ ABI.
3. Run the pass only when the ABI is Itanium.

Originally I thought we could write a generic transformation that might work for the MS ABI as well as the Itanium, but after abit of diggin around I'm much more inclined to keep this pass Itanium-only, and extend it in the future to the MS ABI.

Thanks for reviewing.


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

https://reviews.llvm.org/D84999