Re: Remove vtable_method field in cgraph_node

[Richard Guenther <rguenther@suse.de>]

On Sun, 17 Apr 2011, Jan Hubicka wrote:

> 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.

At some point we dropped all OBJ_TYPE_REF with direct fn but that lead to
issues.  I'll try to dig up what that was.

Richard.

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

-- 
Richard Guenther <rguenther@suse.de>
Novell / SUSE Labs
SUSE LINUX Products GmbH - Nuernberg - AG Nuernberg - HRB 16746 - GF: Markus Rex