diff mbox

Fix -fcompare-debug due to incorrect block pruning (PR c++/70594)

Message ID 20160414183132.GG19207@tucnak.redhat.com
State New
Headers show

Commit Message

Jakub Jelinek April 14, 2016, 6:31 p.m. UTC
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?

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.


	Jakub

Comments

Richard Biener April 14, 2016, 7:15 p.m. UTC | #1
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
diff mbox

Patch

--- 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);