public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* Remove vtable_method field in cgraph_node
@ 2011-04-17 17:26 Jan Hubicka
  2011-04-17 21:27 ` Richard Guenther
  2011-04-20 13:43 ` Martin Jambor
  0 siblings, 2 replies; 8+ messages in thread
From: Jan Hubicka @ 2011-04-17 17:26 UTC (permalink / raw)
  To: gcc-patches, rguenther, mjambor

Hi,
this patch drops vtable_method filed. I never understood what it is about but reading
PR20991 I am convinced that we hit the same problem with work on new devirutalization code.
I implemented what Mark describe as "ideal solution" there - i.e. teaching cgraph that
virtual functions might become reachable until after inlining.  Since we still can
devirutalize in late compilation (that is pretty much poinless but anyway), we no
ahve can_refer_decl_in_current_unit_p that tests if the function has been already thrown
away.  Perhaps we might apply there the observation about vtable being output somewhere,
but I do not think it is safe: if vtable is COMDAT, we can pretty much also optimize
all references to it out in all compilation unit and up not outputting it.
When vtable is not COMDAT, the methods won't be either and this trick will not apply.

Consequently I am dropping the flag. This is very trivial based on observation
that cp_fold_obj_type_ref, the only setter of the flag, is now dead. Plus the
varasm code is no-longer executed at the time of IPA optimizations that the original
patch was fixing.

Martin,
can you please look into why cp_fold_obj_type_ref is no longer used and if possible
get rid of it?

Richi,
compiling the original testcase:
// PR middle-end/20991
// { dg-options "-O2" }
// { dg-do compile }

struct S
{
  virtual inline int foo () const;
  virtual inline bool bar () const;
  virtual int baz (int) const;
};

inline int S::foo () const
{
  return 1;
}

inline bool S::bar () const
{
  return foo () == 0;
}

void A ()
{
  S s;
  if (s.bar ())
    s.foo ();
}

void B ()
{
  S s;
  if (s.bar ())
    s.foo ();
}

we end up with bit amusing code:
void B() ()
{ 
  int D.2147;
  struct S s;

<bb 2>:
  s._vptr.S = &_ZTV1S[2];
  D.2147_8 = OBJ_TYPE_REF(foo;&s->0) (&s);
  return;

}

notice that OBJ_TYPE_REF is useless since foo got devitualized and it holds alive
the useless s._vptr.S = &_ZTV1S[2].  We want to drop OBJ_TYPE_REF at time we fold
first argument to constant.

a.C.027t.fre1 dump reads:
void B() ()
{
  int (*__vtbl_ptr_type) () * D.2149;
  int (*__vtbl_ptr_type) () D.2148;
  int D.2147;
  bool D.2146;
  struct S s;

<bb 2>:
  s._vptr.S = &_ZTV1S[2];
  D.2149_6 = &_ZTV1S[2];
  D.2148_7 = foo;
  D.2147_8 = OBJ_TYPE_REF(D.2148_7;&s->0) (&s);
  D.2146_9 = D.2147_8 == 0;
  D.2146_12 = D.2146_9;
  D.2146_1 = D.2146_12;
  D.2146_2 = D.2146_1;
  return;

}

already I would expect FRE to replace D.2148_7 by foo.  Later this is done by copyprop1

Folding statement: D.2147_8 = OBJ_TYPE_REF(D.2148_7;&s->0) (&s);
Folded into: D.2147_8 = OBJ_TYPE_REF(foo;&s->0) (&s);

void B() ()
{
  int (*__vtbl_ptr_type) () * D.2149;
  int (*__vtbl_ptr_type) () D.2148;
  int D.2147;
  bool D.2146;
  struct S s;

<bb 2>:
  s._vptr.S = &_ZTV1S[2];
  D.2147_8 = OBJ_TYPE_REF(foo;&s->0) (&s);
  return;

}

When copyprop chose to do constant propagation for whatever reason, it probably should
do the folding, too. Either in substitute_and_fold or via its own ccp_fold_stmt.  I am sure
you know the best alternatives better than I  do ;)

Honza

	* cgraph.c (cgraph_clone_node): Do not handle vtable_method
	* cgraph.h (struct cgraph_local_info): Drop vtable_method.
	* cgraphunit.c (cgraph_copy_node_for_versioning): Drop vtable_method.
	* lto-cgraph.c (lto_output_node, input_overwrite_node): Drop vtable method.
	* gimple-fold.c (can_refer_decl_in_current_unit_p): Mention PR20991 in
	gimple-fold.c
	* varasm.c (mark_decl_referenced): Drop vtable_method handling code.
	* cp/class.c (cp_fold_obj_type_ref): Drop vtable_method.
Index: cgraph.c
===================================================================
*** cgraph.c	(revision 172608)
--- cgraph.c	(working copy)
*************** cgraph_clone_node (struct cgraph_node *n
*** 2161,2167 ****
    new_node->local = n->local;
    new_node->local.externally_visible = false;
    new_node->local.local = true;
-   new_node->local.vtable_method = false;
    new_node->global = n->global;
    new_node->rtl = n->rtl;
    new_node->count = count;
--- 2161,2166 ----
Index: cgraph.h
===================================================================
*** cgraph.h	(revision 172608)
--- cgraph.h	(working copy)
*************** struct GTY(()) cgraph_local_info {
*** 95,104 ****
    /* True when the function has been originally extern inline, but it is
       redefined now.  */
    unsigned redefined_extern_inline : 1;
- 
-   /* True if the function is going to be emitted in some other translation
-      unit, referenced from vtable.  */
-   unsigned vtable_method : 1;
  };
  
  /* Information about the function that needs to be computed globally
--- 95,100 ----
Index: cgraphunit.c
===================================================================
*** cgraphunit.c	(revision 172608)
--- cgraphunit.c	(working copy)
*************** cgraph_copy_node_for_versioning (struct 
*** 1991,1997 ****
     new_version->local = old_version->local;
     new_version->local.externally_visible = false;
     new_version->local.local = true;
-    new_version->local.vtable_method = false;
     new_version->global = old_version->global;
     new_version->rtl = old_version->rtl;
     new_version->reachable = true;
--- 1991,1996 ----
Index: cp/class.c
===================================================================
*** cp/class.c	(revision 172608)
--- cp/class.c	(working copy)
*************** cp_fold_obj_type_ref (tree ref, tree kno
*** 8402,8409 ****
  				  DECL_VINDEX (fndecl)));
  #endif
  
-   cgraph_get_node (fndecl)->local.vtable_method = true;
- 
    return build_address (fndecl);
  }
  
--- 8402,8407 ----
Index: lto-cgraph.c
===================================================================
*** lto-cgraph.c	(revision 172608)
--- lto-cgraph.c	(working copy)
*************** lto_output_node (struct lto_simple_outpu
*** 491,497 ****
    bp_pack_value (&bp, node->local.finalized, 1);
    bp_pack_value (&bp, node->local.can_change_signature, 1);
    bp_pack_value (&bp, node->local.redefined_extern_inline, 1);
-   bp_pack_value (&bp, node->local.vtable_method, 1);
    bp_pack_value (&bp, node->needed, 1);
    bp_pack_value (&bp, node->address_taken, 1);
    bp_pack_value (&bp, node->abstract_and_needed, 1);
--- 491,496 ----
*************** input_overwrite_node (struct lto_file_de
*** 927,933 ****
    node->local.finalized = bp_unpack_value (bp, 1);
    node->local.can_change_signature = bp_unpack_value (bp, 1);
    node->local.redefined_extern_inline = bp_unpack_value (bp, 1);
-   node->local.vtable_method = bp_unpack_value (bp, 1);
    node->needed = bp_unpack_value (bp, 1);
    node->address_taken = bp_unpack_value (bp, 1);
    node->abstract_and_needed = bp_unpack_value (bp, 1);
--- 926,931 ----
Index: gimple-fold.c
===================================================================
*** gimple-fold.c	(revision 172608)
--- gimple-fold.c	(working copy)
*************** can_refer_decl_in_current_unit_p (tree d
*** 80,86 ****
      return true;
    /* We are not at ltrans stage; so don't worry about WHOPR.
       Also when still gimplifying all referred comdat functions will be
!      produced.  */
    if (!flag_ltrans && (!DECL_COMDAT (decl) || !cgraph_function_flags_ready))
      return true;
    /* If we already output the function body, we are safe.  */
--- 80,89 ----
      return true;
    /* We are not at ltrans stage; so don't worry about WHOPR.
       Also when still gimplifying all referred comdat functions will be
!      produced.
!      ??? as observed in PR20991 for already optimized out comdat virtual functions
!      we may not neccesarily give up because the copy will be output elsewhere when
!      corresponding vtable is output.  */
    if (!flag_ltrans && (!DECL_COMDAT (decl) || !cgraph_function_flags_ready))
      return true;
    /* If we already output the function body, we are safe.  */
Index: varasm.c
===================================================================
*** varasm.c	(revision 172608)
--- varasm.c	(working copy)
*************** mark_decl_referenced (tree decl)
*** 2201,2208 ****
  	 definition.  */
        struct cgraph_node *node = cgraph_get_create_node (decl);
        if (!DECL_EXTERNAL (decl)
! 	  && (!node->local.vtable_method || !cgraph_global_info_ready
! 	      || !node->local.finalized))
  	cgraph_mark_needed_node (node);
      }
    else if (TREE_CODE (decl) == VAR_DECL)
--- 2201,2207 ----
  	 definition.  */
        struct cgraph_node *node = cgraph_get_create_node (decl);
        if (!DECL_EXTERNAL (decl)
! 	  && !node->local.finalized)
  	cgraph_mark_needed_node (node);
      }
    else if (TREE_CODE (decl) == VAR_DECL)

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2011-04-20 13:24 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-04-17 17:26 Remove vtable_method field in cgraph_node Jan Hubicka
2011-04-17 21:27 ` Richard Guenther
2011-04-18  0:51   ` Jan Hubicka
2011-04-18  9:27     ` Richard Guenther
2011-04-18 15:40       ` Jan Hubicka
2011-04-18 15:41         ` Richard Guenther
2011-04-18 16:27           ` Jan Hubicka
2011-04-20 13:43 ` Martin Jambor

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).