Re: [WIP] Avoid -fcompare-debug regressions due to ipa-polymorphic-call stuff (PR ipa/65610)

[Jakub Jelinek <jakub at redhat dot com>]

On Mon, Mar 30, 2015 at 08:45:51PM +0200, Jan Hubicka wrote:
> We do another round of polymorphic call analysis in PRE pass. (it is not that
> important to do so as it is too late to inline the call, but we do that)

Ok.

> > Also, looking for better name of the function if you have ideas.
> 
> I think inlined_polymorphic_ctor_dtor_block_p may be descriptive name? :)

Ack.

> Looking at it, the function is unnecesarily conservative - it can rule out all
> non-polymorphic cdtors.  I will drop that to my TODO for next stage1.

Yeah, will leave that to you.

> There is one extra occurence of the same code in param_type_may_change_p.
> Please unify it, too.  I should not have introduced the duplicates at first place
> but initial version of the code had those all different.

Done.

> > --- gcc/tree-ssa-live.c.jj	2015-03-09 08:05:13.000000000 +0100
> > +++ gcc/tree-ssa-live.c	2015-03-30 11:57:25.492574441 +0200
> > @@ -76,6 +76,10 @@ along with GCC; see the file COPYING3.
> >  #include "diagnostic-core.h"
> >  #include "debug.h"
> >  #include "tree-ssa.h"
> > +#include "lto-streamer.h"
> > +#include "ipa-ref.h"
> > +#include "cgraph.h"
> > +#include "ipa-utils.h"
> 
> Header flattening still makes me sad ;(

I still fail to understand what is the advantage of doing that, the info
on what headers are really needed and what are grabbed manually because of
dependencies or worse just cut-n-pasted from elsewhere in a hope that things
will finally on 20th attempt compile.

> Please drop the cfun->after_inlining check.
> I do not think it is major problem - the blocks of inlined cdtors should not be that
> common (we can alternatively add the check for the polymorphic type in the predicate
> above - polymorphic cdtors are even less common, but I doubt that is going to make
> memory use difference).
> 
> Patch is OK with these changes.

Due to the longer function name, had to do a small change:
+  bool check_clones = !base || is_global_var (base);
   for (tree block = gimple_block (call); block && TREE_CODE (block) == BLOCK;
        block = BLOCK_SUPERCONTEXT (block))
-    if (BLOCK_ABSTRACT_ORIGIN (block)
-       && TREE_CODE (BLOCK_ABSTRACT_ORIGIN (block)) == FUNCTION_DECL)
+    if (tree fn = inlined_polymorphic_ctor_dtor_block_p (block, check_clones))

hope that is ok (base really shouldn't change during the loop, so we will
even safe some calls to is_global_var).

Here is what I'm going to re-bootstrap/re-regtest now.

2015-03-30  Jakub Jelinek  <jakub@redhat.com>

	PR ipa/65610
	* ipa-utils.h (inlined_polymorphic_ctor_dtor_block_p): Declare.
	* ipa-polymorphic-call.c (inlined_polymorphic_ctor_dtor_block_p): New
	function.
	(decl_maybe_in_construction_p, noncall_stmt_may_be_vtbl_ptr_store):
	Use it.
	* ipa-prop.c (param_type_may_change_p): Likewise.
	* tree-ssa-live.c: Include ipa-utils.h and its dependencies.
	(remove_unused_scope_block_p): Add in_ctor_dtor_block
	argument.  Before inlining, preserve
	inlined_polymorphic_ctor_dtor_block_p blocks and the outermost block
	with FUNCTION_DECL BLOCK_ABSTRACT_ORIGIN inside of them.  Adjust
	recursive calls.
	(remove_unused_locals): Adjust remove_unused_scope_block_p caller.

	* g++.dg/ubsan/pr65610.C: New test.

--- gcc/ipa-utils.h.jj	2015-03-30 12:18:16.578355523 +0200
+++ gcc/ipa-utils.h	2015-03-30 21:00:55.549241356 +0200
@@ -70,6 +70,7 @@ bool possible_polymorphic_call_target_p
 				         const ipa_polymorphic_call_context &,
 					 struct cgraph_node *);
 tree method_class_type (const_tree);
+tree inlined_polymorphic_ctor_dtor_block_p (tree, bool);
 bool decl_maybe_in_construction_p (tree, tree, gimple, tree);
 tree vtable_pointer_value_to_binfo (const_tree);
 bool vtable_pointer_value_to_vtable (const_tree, tree *, unsigned HOST_WIDE_INT *);
--- gcc/ipa-polymorphic-call.c.jj	2015-03-30 12:18:16.494356880 +0200
+++ gcc/ipa-polymorphic-call.c	2015-03-30 21:02:56.895297752 +0200
@@ -513,6 +513,38 @@ contains_type_p (tree outer_type, HOST_W
 }
 
 
+/* Return a FUNCTION_DECL if BLOCK represents a constructor or destructor.
+   If CHECK_CLONES is true, also check for clones of ctor/dtors.  */
+
+tree
+inlined_polymorphic_ctor_dtor_block_p (tree block, bool check_clones)
+{
+  tree fn = BLOCK_ABSTRACT_ORIGIN (block);
+  if (fn == NULL || TREE_CODE (fn) != FUNCTION_DECL)
+    return NULL_TREE;
+
+  if (TREE_CODE (TREE_TYPE (fn)) != METHOD_TYPE
+      || (!DECL_CXX_CONSTRUCTOR_P (fn) && !DECL_CXX_DESTRUCTOR_P (fn)))
+    {
+      if (!check_clones)
+	return NULL_TREE;
+
+      /* Watch for clones where we constant propagated the first
+	 argument (pointer to the instance).  */
+      fn = DECL_ABSTRACT_ORIGIN (fn);
+      if (!fn
+	  || TREE_CODE (TREE_TYPE (fn)) != METHOD_TYPE
+	  || (!DECL_CXX_CONSTRUCTOR_P (fn) && !DECL_CXX_DESTRUCTOR_P (fn)))
+	return NULL_TREE;
+    }
+
+  if (flags_from_decl_or_type (fn) & (ECF_PURE | ECF_CONST))
+    return NULL_TREE;
+
+  return fn;
+}
+
+
 /* We know that the instance is stored in variable or parameter
    (not dynamically allocated) and we want to disprove the fact
    that it may be in construction at invocation of CALL.
@@ -550,30 +582,11 @@ decl_maybe_in_construction_p (tree base,
       && flags_from_decl_or_type (function) & (ECF_PURE | ECF_CONST))
     return false;
 
+  bool check_clones = !base || is_global_var (base);
   for (tree block = gimple_block (call); block && TREE_CODE (block) == BLOCK;
        block = BLOCK_SUPERCONTEXT (block))
-    if (BLOCK_ABSTRACT_ORIGIN (block)
-	&& TREE_CODE (BLOCK_ABSTRACT_ORIGIN (block)) == FUNCTION_DECL)
+    if (tree fn = inlined_polymorphic_ctor_dtor_block_p (block, check_clones))
       {
-	tree fn = BLOCK_ABSTRACT_ORIGIN (block);
-
-	if (TREE_CODE (TREE_TYPE (fn)) != METHOD_TYPE
-	    || (!DECL_CXX_CONSTRUCTOR_P (fn)
-		&& !DECL_CXX_DESTRUCTOR_P (fn)))
-	  {
-	    /* Watch for clones where we constant propagated the first
-	       argument (pointer to the instance).  */
-	    fn = DECL_ABSTRACT_ORIGIN (fn);
-	    if (!fn
-		|| (base && !is_global_var (base))
-	        || TREE_CODE (TREE_TYPE (fn)) != METHOD_TYPE
-		|| (!DECL_CXX_CONSTRUCTOR_P (fn)
-		    && !DECL_CXX_DESTRUCTOR_P (fn)))
-	      continue;
-	  }
-	if (flags_from_decl_or_type (fn) & (ECF_PURE | ECF_CONST))
-	  continue;
-
 	tree type = TYPE_MAIN_VARIANT (method_class_type (TREE_TYPE (fn)));
 
 	if (!outer_type || !types_odr_comparable (type, outer_type))
@@ -1163,15 +1176,7 @@ noncall_stmt_may_be_vtbl_ptr_store (gimp
        block = BLOCK_SUPERCONTEXT (block))
     if (BLOCK_ABSTRACT_ORIGIN (block)
 	&& TREE_CODE (BLOCK_ABSTRACT_ORIGIN (block)) == FUNCTION_DECL)
-      {
-	tree fn = BLOCK_ABSTRACT_ORIGIN (block);
-
-	if (flags_from_decl_or_type (fn) & (ECF_PURE | ECF_CONST))
-	  return false;
-	return (TREE_CODE (TREE_TYPE (fn)) == METHOD_TYPE
-		&& (DECL_CXX_CONSTRUCTOR_P (fn)
-		    || DECL_CXX_DESTRUCTOR_P (fn)));
-      }
+      return inlined_polymorphic_ctor_dtor_block_p (block, false);
   return (TREE_CODE (TREE_TYPE (current_function_decl)) == METHOD_TYPE
 	  && (DECL_CXX_CONSTRUCTOR_P (current_function_decl)
 	      || DECL_CXX_DESTRUCTOR_P (current_function_decl)));
--- gcc/ipa-prop.c.jj	2015-02-26 13:23:51.000000000 +0100
+++ gcc/ipa-prop.c	2015-03-30 21:03:40.681595669 +0200
@@ -715,18 +715,8 @@ param_type_may_change_p (tree function,
 	  /* Walk the inline stack and watch out for ctors/dtors.  */
 	  for (tree block = gimple_block (call); block && TREE_CODE (block) == BLOCK;
 	       block = BLOCK_SUPERCONTEXT (block))
-	    if (BLOCK_ABSTRACT_ORIGIN (block)
-	        && TREE_CODE (BLOCK_ABSTRACT_ORIGIN (block)) == FUNCTION_DECL)
-	      {
-		tree fn = BLOCK_ABSTRACT_ORIGIN (block);
-
-		if (flags_from_decl_or_type (fn) & (ECF_PURE | ECF_CONST))
-		  continue;
-		if (TREE_CODE (TREE_TYPE (fn)) == METHOD_TYPE
-		    && (DECL_CXX_CONSTRUCTOR_P (fn)
-		        || DECL_CXX_DESTRUCTOR_P (fn)))
-		  return true;
-	      }
+	    if (inlined_polymorphic_ctor_dtor_block_p (block, false))
+	      return true;
 	  return false;
 	}
     }
--- gcc/tree-ssa-live.c.jj	2015-03-30 12:18:16.545356056 +0200
+++ gcc/tree-ssa-live.c	2015-03-30 21:00:55.551241324 +0200
@@ -76,6 +76,10 @@ along with GCC; see the file COPYING3.
 #include "diagnostic-core.h"
 #include "debug.h"
 #include "tree-ssa.h"
+#include "lto-streamer.h"
+#include "ipa-ref.h"
+#include "cgraph.h"
+#include "ipa-utils.h"
 
 #ifdef ENABLE_CHECKING
 static void  verify_live_on_entry (tree_live_info_p);
@@ -509,12 +513,29 @@ mark_scope_block_unused (tree scope)
    done by the inliner.  */
 
 static bool
-remove_unused_scope_block_p (tree scope)
+remove_unused_scope_block_p (tree scope, bool in_ctor_dtor_block)
 {
   tree *t, *next;
   bool unused = !TREE_USED (scope);
   int nsubblocks = 0;
 
+  /* For ipa-polymorphic-call.c purposes, preserve blocks:
+     1) with BLOCK_ABSTRACT_ORIGIN of a ctor/dtor or their clones  */
+  if (inlined_polymorphic_ctor_dtor_block_p (scope, true))
+    {
+      in_ctor_dtor_block = true;
+      unused = false;
+    }
+  /* 2) inside such blocks, the outermost block with BLOCK_ABSTRACT_ORIGIN
+     being a FUNCTION_DECL.  */
+  else if (in_ctor_dtor_block
+	   && BLOCK_ABSTRACT_ORIGIN (scope)
+	   && TREE_CODE (BLOCK_ABSTRACT_ORIGIN (scope)) == FUNCTION_DECL)
+    {
+      in_ctor_dtor_block = false;
+      unused = false;
+    }
+
   for (t = &BLOCK_VARS (scope); *t; t = next)
     {
       next = &DECL_CHAIN (*t);
@@ -594,7 +615,7 @@ remove_unused_scope_block_p (tree scope)
     }
 
   for (t = &BLOCK_SUBBLOCKS (scope); *t ;)
-    if (remove_unused_scope_block_p (*t))
+    if (remove_unused_scope_block_p (*t, in_ctor_dtor_block))
       {
 	if (BLOCK_SUBBLOCKS (*t))
 	  {
@@ -959,7 +980,7 @@ remove_unused_locals (void)
       cfun->local_decls->truncate (dstidx);
     }
 
-  remove_unused_scope_block_p (DECL_INITIAL (current_function_decl));
+  remove_unused_scope_block_p (DECL_INITIAL (current_function_decl), false);
   clear_unused_block_pointer ();
 
   BITMAP_FREE (usedvars);
--- gcc/testsuite/g++.dg/ubsan/pr65610.C.jj	2015-03-30 21:00:55.552241308 +0200
+++ gcc/testsuite/g++.dg/ubsan/pr65610.C	2015-03-30 21:00:55.552241308 +0200
@@ -0,0 +1,57 @@
+// PR ipa/65610
+// { dg-do compile }
+// { dg-options "-std=c++11 -fsanitize=undefined -fno-sanitize=vptr -fcompare-debug" }
+
+class A;
+class B {};
+enum C { D };
+class E;
+class F;
+class G;
+class H
+{
+  F m1 (const A &t) const;
+  G m2 () const;
+};
+class G {};
+template <class S, class T>
+class I;
+template <class S, class T>
+class J
+{
+  friend class I <S,T>;
+  J <S,T> *j;
+};
+template <class S, class T>
+struct I
+{
+  virtual ~I ();
+  virtual void m3 (void *p) {}
+  J <S,T> *i;
+  void m4 (J <S,T>*& t);
+};
+template <class S, class T>
+void I <S,T>::m4 (J <S,T> * &t)
+{
+  m4 (t->j);
+  m3 (t);
+}
+template <class S, class T>
+I <S,T>::~I ()
+{
+  m4 (i);
+}
+struct F
+{
+  explicit inline F (C v);
+  inline ~F ();
+  I <B, E> f;
+};
+inline F::F (C v) {}
+inline F::~F () {}
+F H::m1 (const A &t) const
+{
+  F q (D);
+  G r = m2 ();
+  return q;
+}


	Jakub