Re: C++ PATCH for c++/58678 (devirt vs. KDE)

[Jan Hubicka <hubicka at ucw dot cz>]

> Honza suggested that if the destructor for an abstract class can't
> ever be called through the vtable, the front end could avoid
> referring to it from the vtable.  This patch replaces such a
> destructor with __cxa_pure_virtual in the vtable.
> 
> Tested x86_64-pc-linux-gnu, applying to trunk.

Thank you!  would preffer different marker than cxa_pure_virtual in the vtable,
most probably simply NULL.

The reason is that __cxa_pure_virtual will appear as a possible target in the
list and it will prevent devirtualiztion to happen when we end up with
__cxa_pure_virtual and real destructor in the list of possible targets.
gimple_get_virt_method_for_vtable knows that lookup in vtable that do not
result in FUNCTION_DECL should be translated to BUILTIN_UNREACHABLE and
ipa-devirt drops these from list of targets, unlike __cxa_pure_virtual that
stays.

Other problem with cxa_pure_virtual is that it needs external relocation.
I sort of wonedered if we don't want to produce hidden comdat wrapper for
it, so C++ programs are easier to relocate.

I will still keep the patch to mark ABSTACT classes by BINFO flag and will
send out the patch I made to make ABSTRACT classes to be ignored for
anonymous namespace types.  It seems to make difference for libreoffice
that uses a lot of abstracts.

What do you think of the following patch that makes ipa-devirt to conclude
that destructor calls are never done on types in construction.
If effect of doing so is undefined, I think it is safe to drop them from
list of targets and that really helps to reduce lists down.


Index: ipa-devirt.c
===================================================================
--- ipa-devirt.c	(revision 208492)
+++ ipa-devirt.c	(working copy)
@@ -1511,7 +1558,10 @@ possible_polymorphic_call_targets (tree
       target = NULL;
     }
 
-  maybe_record_node (nodes, target, inserted, can_refer, &complete);
+  /* Destructors are never called through construction virtual tables,
+     because the type is always known.  */
+  if (target && DECL_CXX_DESTRUCTOR_P (target))
+    context.maybe_in_construction = false;
 
   if (target)
     {