Hey, I am indeed looking for feedback. Thanks for linking the thread, it is indeed about the same as what I am trying to achieve.
However, my issue with Jason's suggestion (I've talked about is a bit here too: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63164) is that it's simply not an optimal solution because of an additional comparison that is not needed, and leaves room for further optimization. You are right about testcases though, I'll start working on those. Also thanks for the tips about e-mail formatting, I'll see what I can do in my next RFC. ________________________________ From: David Malcolm <dmalc...@redhat.com> Sent: 28 July 2025 22:16:03 To: Thomas de Bock; gcc-patches@gcc.gnu.org Subject: [ext] Re: [RFC] c++: Optimize out dynamic_cast call when target is final [PR63164] On Mon, 2025-07-28 at 16:50 +0000, Thomas de Bock wrote: > This patch optimizes out calls to __dynamic_cast when the type being > cast to is final (or its destructor), replacing them with a simple > comparison of the types' vtable addresses. This is already > implemented and done by default in clang > (https://urldefense.com/v3/__https://reviews.llvm.org/D154658__;!!EvhwMw!RZ5nFEkGb0w_xHwtmVN-esSX8oeIuZEfSpyq0CaMVTL3OfveQyaWC_req4FtYdlI7H8atbyQcQp_pNf3$ > ), but can be turned off with -fno- > assume-unique-vtables, due to the problems it can cause with unmerged > vtables when using shared libraries. With the optimization not really > fitting well under any existing flag (that I know of), I believe an - > fassume-unique-vtables flag (or something similar) would be > appropriate to add to gcc, which besides enabling this optimization, > should probably also define __GXX_MERGED_TYPEINFO_NAMES as 1. > > diff --git a/gcc/cp/rtti.cc b/gcc/cp/rtti.cc > index c06a18b3ff1..a4259253f8d 100644 > --- a/gcc/cp/rtti.cc > +++ b/gcc/cp/rtti.cc > @@ -761,6 +761,51 @@ build_dynamic_cast_1 (location_t loc, tree type, > tree expr, > if (tc == REFERENCE_TYPE) > expr1 = cp_build_addr_expr (expr1, complain); > > + > + > + /* If type is final, don't call dynamic_cast. > + * Instead just check vtable equivalence at runtime. > + * TYPE_FINAL_P does not return true for non-final class with > + * final destructor overriding virtual though, > + * so look through virtual functions for final destructor */ > + > + bool can_inherit = !TYPE_FINAL_P (target_type); > + tree vchain; > + for (vchain = BINFO_VIRTUALS (TYPE_BINFO (target_type)); > + vchain && can_inherit; > + vchain = TREE_CHAIN (vchain)) > + { > + if (!DECL_DESTRUCTOR_P (BV_FN (vchain))) > + continue; > + if (!DECL_FINAL_P (BV_FN (vchain))) > + continue; > + can_inherit = false; > + } > + > + if (!can_inherit) > + { > + /* Retrieve vtable declaration and address. */ > + tree trgt_vtbl_addr = build_address (get_vtable_decl > (target_type, 0)); > + /* The offset-to-top field is at index -2 from the vptr. > + * Offsetting -2 on build_vtbl_ref (at low optimization) > results in register being > + * subtracted from at runtime, done on vtbl address to > prevent */ > + tree trgt_vptr_addr = build2 (POINTER_PLUS_EXPR, > TREE_TYPE(trgt_vtbl_addr), > + trgt_vtbl_addr, size_int(2 * > TARGET_VTABLE_DATA_ENTRY_DISTANCE * > + tree_to_shwi (TYPE_SIZE(ptr_type_node))/8)); > + > + tree src_obj = cp_build_fold_indirect_ref (expr); > + tree src_vptr = build_address (build_vtbl_ref (src_obj, > size_int(0))); > + > + /* Check vtable equivalence by vptr address */ > + tree cond = build2 (NE_EXPR, boolean_type_node, > trgt_vptr_addr, src_vptr); > + tree result = build3 (COND_EXPR, type, cond, nullptr_node, > + build_static_cast (loc, type, expr, > complain)); > + SET_EXPR_LOCATION (result, loc); > + result = cp_convert (type, result, complain); > + > + return build_if_nonnull (expr, result, complain); > + } > + > elems[0] = expr1; > elems[1] = td3; > elems[2] = td2; > > > (Resent as I do not have write access so would appreciate someone > taking a look, original msg: > https://urldefense.com/v3/__https://gcc.gnu.org/pipermail/gcc-patches/2025-July/690058.html__;!!EvhwMw!RZ5nFEkGb0w_xHwtmVN-esSX8oeIuZEfSpyq0CaMVTL3OfveQyaWC_req4FtYdlI7H8atbyQcXZpDKmk$ > ) Hi Thomas I'm not knowledgeable enough about the C++ frontend to review your patch, but I'd expect the reviewer to want some kind of test coverage with cases that trigger the optimization and verify the generated code. So I'd expect such a patch to also touch gcc/testsuite/g++.dg Please use "git format-patch" to generate emails, and please add a sign-off line to them. There are some notes on what makes a good patch at https://urldefense.com/v3/__https://gcc.gnu.org/contribute.html__;!!EvhwMw!RZ5nFEkGb0w_xHwtmVN-esSX8oeIuZEfSpyq0CaMVTL3OfveQyaWC_req4FtYdlI7H8atbyQcVzDA53y$ Am I right in thinking from the "RFC" in the subject line that the patch is something of a work-in-progress and you're hoping for feedback from the C++ maintainers on whether the approach is correct? I noticed that Daniel Bertalan has also posted a couple of patches for this issue (which FWIW do have test cases); see: https://urldefense.com/v3/__https://gcc.gnu.org/pipermail/gcc-patches/2025-May/684277.html__;!!EvhwMw!RZ5nFEkGb0w_xHwtmVN-esSX8oeIuZEfSpyq0CaMVTL3OfveQyaWC_req4FtYdlI7H8atbyQcfSfcXDX$ and https://urldefense.com/v3/__https://gcc.gnu.org/pipermail/gcc-patches/2025-June/686362.html__;!!EvhwMw!RZ5nFEkGb0w_xHwtmVN-esSX8oeIuZEfSpyq0CaMVTL3OfveQyaWC_req4FtYdlI7H8atbyQcaLqMMms$ and Jason's replies to those. > > This e-mail and any attachments may contain information that is > confidential and proprietary and otherwise protected from disclosure. > If you are not the intended recipient of this e-mail, do not read, > duplicate or redistribute it by any means. Please immediately delete > it and any attachments and notify the sender that you have received > it by mistake. Unintended recipients are prohibited from taking > action on the basis of information in this e-mail or any attachments. > The DRW Companies make no representations that this e-mail or any > attachments are free of computer viruses or other defects. I wonder if there's a way you can turn off this footer when posting to the mailing list? The mailing list is public, and thus presumably everyone in the world is an intended recipient, so maybe much of the above is thus rendered a no-op? (though I am not a lawyer). Thanks again for posting the patch; hope this is constructive feedback Dave
