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 <rguent...@suse.de>
Novell / SUSE Labs
SUSE LINUX Products GmbH - Nuernberg - AG Nuernberg - HRB 16746 - GF: Markus Rex
