Re: [PATCH] Fix -fcompare-debug due to incorrect block pruning (PR c++/70594)

[Richard Biener <rguenther at suse dot de>]

On April 14, 2016 8:31:32 PM GMT+02:00, Jakub Jelinek <jakub@redhat.com> wrote:
>Hi!
>
>On a testcase Tobias provided privately there is a -fcompare-debug
>failure due to different cgraph node->order values in the printout.
>The problem is in different result from the
>noncall_stmt_may_be_vtbl_ptr_store function on a store, which is caused
>by too aggressive block pruning with -g0.
>
>The noncall_stmt_may_be_vtbl_ptr_store relies on enough BLOCKS to be
>preserved, such that if block_ultimate_origin of some BLOCK
>(gimple_block or
>up through its BLOCK_SUPERCONTEXTs) is a FUNCTION_DECL, then it is
>a cdtor if the stmt is originally inlined from a cdtor, or is some
>other FUNCTION_DECL if it is not originally inlined from a cdtor.
>To achieve that, tree-ssa-live.c attempts to keep the blocks
>with block_ultimate_origin of FUNCTION_DECL which is cdtor,
>and nearest block with block_ultimate_origin of FUNCTION_DECL
>that is not cdtor below the cdtor one.
>
>There is:
>  /* 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;
>    }
>which is roughly right, but there are 2 bugs, both of which affect
>the testcase I've looked at.
>
>1) the above works well if a non-cdtor function is inlined into cdtor
>and the cdtor is then inlined into some other function and pruning
>happens
>only afterwards, but if a non-cdtor function is inlined into cdtor,
>then
>pruning happens on the cdtor, the block with block_ultimate_origin
>of FUNCTION_DECL can be pruned, and then finally the cdtor function is
>inlined into some other one and noncall_stmt_may_be_vtbl_ptr_store
>depends
>on -g
>
>2) another issue is that noncall_stmt_may_be_vtbl_ptr_store uses
>block_ultimate_origin, and for the cdtor blocks we use it in
>remove_unused_scope_block_p too (because
>inlined_polymorphic_ctor_dtor_block_p calls it), but for the other
>blocks we just check BLOCK_ABSTRACT_ORIGIN if it is FUNCTION_DECL.
>If it isn't a FUNCTION_DECL, but block_ultimate_origin is
>FUNCTION_DECL,
>we still prune it, even when we shouldn't.
>
>Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

OK.

Thanks,
Richard.

>2016-04-14  Jakub Jelinek  <jakub@redhat.com>
>
>	PR c++/70594
>	* ipa-utils.h (polymorphic_ctor_dtor_p): New prototype.
>	* ipa-polymorphic-call.c (polymorphic_ctor_dtor_p): New function.
>	(inlined_polymorphic_ctor_dtor_block_p): Use it.
>	* tree-ssa-live.c (remove_unused_scope_block_p): When
>	in_ctor_dtor_block, avoid discarding not just BLOCKs with
>	BLOCK_ABSTRACT_ORIGIN being FUNCTION_DECL, but even when
>	block_ultimate_origin is FUNCTION_DECL.
>	(remove_unused_locals): If current_function_decl is
>	polymorphic_ctor_dtor_p, pass initial true to
>	remove_unused_scope_block_p' is_ctor_dtor_block.
>
>--- gcc/ipa-utils.h.jj	2016-01-04 14:55:51.000000000 +0100
>+++ gcc/ipa-utils.h	2016-04-14 16:46:08.828444152 +0200
>@@ -70,6 +70,7 @@ void dump_possible_polymorphic_call_targ
> bool possible_polymorphic_call_target_p (tree, HOST_WIDE_INT,
> 				         const ipa_polymorphic_call_context &,
> 					 struct cgraph_node *);
>+tree polymorphic_ctor_dtor_p (tree, bool);
> 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);
>--- gcc/ipa-polymorphic-call.c.jj	2016-03-30 16:00:17.000000000 +0200
>+++ gcc/ipa-polymorphic-call.c	2016-04-14 16:45:45.407754387 +0200
>@@ -479,16 +479,12 @@ contains_type_p (tree outer_type, HOST_W
> }
> 
> 
>-/* Return a FUNCTION_DECL if BLOCK represents a constructor or
>destructor.
>+/* Return a FUNCTION_DECL if FN represent 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)
>+polymorphic_ctor_dtor_p (tree fn, bool check_clones)
> {
>-  tree fn = block_ultimate_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)))
>     {
>@@ -510,6 +506,19 @@ inlined_polymorphic_ctor_dtor_block_p (t
>   return fn;
> }
> 
>+/* 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_ultimate_origin (block);
>+  if (fn == NULL || TREE_CODE (fn) != FUNCTION_DECL)
>+    return NULL_TREE;
>+
>+  return polymorphic_ctor_dtor_p (fn, check_clones);
>+}
>+
> 
> /* We know that the instance is stored in variable or parameter
>    (not dynamically allocated) and we want to disprove the fact
>--- gcc/tree-ssa-live.c.jj	2016-01-04 14:55:51.000000000 +0100
>+++ gcc/tree-ssa-live.c	2016-04-14 17:24:49.047509853 +0200
>@@ -393,14 +393,16 @@ remove_unused_scope_block_p (tree scope,
>       in_ctor_dtor_block = true;
>       unused = false;
>     }
>-  /* 2) inside such blocks, the outermost block with
>BLOCK_ABSTRACT_ORIGIN
>+  /* 2) inside such blocks, the outermost block with
>block_ultimate_origin
>      being a FUNCTION_DECL.  */
>-  else if (in_ctor_dtor_block
>-	   && BLOCK_ABSTRACT_ORIGIN (scope)
>-	   && TREE_CODE (BLOCK_ABSTRACT_ORIGIN (scope)) == FUNCTION_DECL)
>+  else if (in_ctor_dtor_block)
>     {
>-      in_ctor_dtor_block = false;
>-      unused = false;
>+      tree fn = block_ultimate_origin (scope);
>+      if (fn && TREE_CODE (fn) == FUNCTION_DECL)
>+	{
>+	  in_ctor_dtor_block = false;
>+	  unused = false;
>+	}
>     }
> 
>   for (t = &BLOCK_VARS (scope); *t; t = next)
>@@ -855,7 +857,9 @@ remove_unused_locals (void)
>       cfun->local_decls->truncate (dstidx);
>     }
> 
>-  remove_unused_scope_block_p (DECL_INITIAL (current_function_decl),
>false);
>+  remove_unused_scope_block_p (DECL_INITIAL (current_function_decl),
>+			       polymorphic_ctor_dtor_p (current_function_decl,
>+							true) != NULL_TREE);
>   clear_unused_block_pointer ();
> 
>   BITMAP_FREE (usedvars);
>
>	Jakub