This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: C++ PATCH for c++/61687 (extra errors with -O2)


> My earlier patch for 61659 caused more virtual functions to be
> instantiated when -fdevirtualize is on, leading to additional errors
> appearing at higher optimization levels.  This patch shifts that
> instantiation to a new flag, -fuse-all-virtuals, which is on by
> default, and adds an explanatory note to help people understand why
> their code is breaking and how they can work around it if needed.
> 
> Tested x86_64-pc-linux-gnu, applying to trunk.

> commit 82187fb06863161765a270f8ba00bdbf975b3af2
> Author: Jason Merrill <jason@redhat.com>
> Date:   Mon Jul 7 03:15:02 2014 -0700
> 
>     	PR c++/61659
>     	PR c++/61687
>     gcc/c-family/
>     	* c.opt (-fuse-all-virtuals): New.
>     gcc/cp/
>     	* decl2.c (mark_all_virtuals): New variable.
>     	(maybe_emit_vtables): Check it instead of flag_devirtualize.
>     	(cp_write_global_declarations): Set it and give helpful diagnostic
>     	if it introduces errors.
>     	* class.c (finish_struct_1): Check it.
>     	* decl.c (grokdeclarator): Clear virtualp after 'virtual auto' error.

Given my experience about numbers of functions that become reachable when you stream all virtuals into LTO,
I wonder if we don't want to use possible_polymorphic_call_targets within the front-end to avoid instantiating
those that can't be called?
I think it should not be too hard - all we need is to populate the type inheritance graph from FE and then
for each polymorphic call produce the list to mark possible targets are reachable.

Honza
> 
> diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt
> index faef774..3a2084f 100644
> --- a/gcc/c-family/c.opt
> +++ b/gcc/c-family/c.opt
> @@ -1268,6 +1268,10 @@ funsigned-char
>  C ObjC C++ ObjC++ LTO Var(flag_signed_char, 0)
>  Make \"char\" unsigned by default
>  
> +fuse-all-virtuals
> +C++ ObjC++ Var(flag_use_all_virtuals) Init(1)
> +Treat all virtual functions as odr-used
> +
>  fuse-cxa-atexit
>  C++ ObjC++ Var(flag_use_cxa_atexit) Init(DEFAULT_USE_CXA_ATEXIT)
>  Use __cxa_atexit to register destructors
> diff --git a/gcc/cp/class.c b/gcc/cp/class.c
> index 3a44dba..d0eb103 100644
> --- a/gcc/cp/class.c
> +++ b/gcc/cp/class.c
> @@ -6408,7 +6408,7 @@ finish_struct_1 (tree t)
>  	 in every translation unit where the class definition appears.  If
>  	 we're devirtualizing, we can look into the vtable even if we
>  	 aren't emitting it.  */
> -      if (CLASSTYPE_KEY_METHOD (t) == NULL_TREE || flag_devirtualize)
> +      if (CLASSTYPE_KEY_METHOD (t) == NULL_TREE || flag_use_all_virtuals)
>  	keyed_classes = tree_cons (NULL_TREE, t, keyed_classes);
>      }
>  
> diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c
> index 1ade586..01d74e3 100644
> --- a/gcc/cp/decl.c
> +++ b/gcc/cp/decl.c
> @@ -9631,8 +9631,11 @@ grokdeclarator (const cp_declarator *declarator,
>  				    "-std=gnu++1y");
>  			  }
>  			else if (virtualp)
> -			  error ("virtual function cannot "
> -				 "have deduced return type");
> +			  {
> +			    error ("virtual function cannot "
> +				   "have deduced return type");
> +			    virtualp = false;
> +			  }
>  		      }
>  		    else if (!is_auto (type))
>  		      {
> diff --git a/gcc/cp/decl2.c b/gcc/cp/decl2.c
> index 98897f4..0926dbc 100644
> --- a/gcc/cp/decl2.c
> +++ b/gcc/cp/decl2.c
> @@ -106,6 +106,11 @@ static GTY(()) vec<tree, va_gc> *no_linkage_decls;
>  /* Nonzero if we're done parsing and into end-of-file activities.  */
>  
>  int at_eof;
> +
> +/* Nonzero if we've instantiated everything used directly, and now want to
> +   mark all virtual functions as used so that they are available for
> +   devirtualization.  */
> +static int mark_all_virtuals;
>  
>  
>  /* Return a member function type (a METHOD_TYPE), given FNTYPE (a
> @@ -2009,7 +2014,7 @@ maybe_emit_vtables (tree ctype)
>        if (DECL_COMDAT (primary_vtbl)
>  	  && CLASSTYPE_DEBUG_REQUESTED (ctype))
>  	note_debug_info_needed (ctype);
> -      if (flag_devirtualize)
> +      if (mark_all_virtuals)
>  	/* Make sure virtual functions get instantiated/synthesized so that
>  	   they can be inlined after devirtualization even if the vtable is
>  	   never emitted.  */
> @@ -4340,6 +4345,8 @@ cp_write_global_declarations (void)
>       instantiated, etc., etc.  */
>  
>    emit_support_tinfos ();
> +  int errs = errorcount + sorrycount;
> +  bool explained_devirt = false;
>  
>    do
>      {
> @@ -4572,6 +4579,27 @@ cp_write_global_declarations (void)
>  					 pending_statics->length ()))
>  	reconsider = true;
>  
> +      if (flag_use_all_virtuals)
> +	{
> +	  if (!reconsider && !mark_all_virtuals)
> +	    {
> +	      mark_all_virtuals = true;
> +	      reconsider = true;
> +	      errs = errorcount + sorrycount;
> +	    }
> +	  else if (mark_all_virtuals
> +		   && !explained_devirt
> +		   && (errorcount + sorrycount > errs))
> +	    {
> +	      inform (global_dc->last_location, "this error is seen due to "
> +		      "instantiation of all virtual functions, which the C++ "
> +		      "standard says are always considered used; this is done "
> +		      "to support devirtualization optimizations, but can be "
> +		      "disabled with -fno-use-all-virtuals");
> +	      explained_devirt = true;
> +	    }
> +	}
> +
>        retries++;
>      }
>    while (reconsider);
> diff --git a/gcc/testsuite/g++.dg/template/dtor9.C b/gcc/testsuite/g++.dg/template/dtor9.C
> index fd71389..006a754 100644
> --- a/gcc/testsuite/g++.dg/template/dtor9.C
> +++ b/gcc/testsuite/g++.dg/template/dtor9.C
> @@ -1,4 +1,5 @@
>  // PR c++/60347
> +// { dg-options "-fno-use-all-virtuals" }
>  
>  struct A;
>  
> diff --git a/gcc/testsuite/g++.dg/template/dtor9a.C b/gcc/testsuite/g++.dg/template/dtor9a.C
> new file mode 100644
> index 0000000..aaae8b6
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/template/dtor9a.C
> @@ -0,0 +1,13 @@
> +// PR c++/60347
> +// { dg-options "-fuse-all-virtuals" }
> +
> +struct A;
> +
> +template <class T>
> +struct B
> +{
> +  T* p;
> +  virtual ~B() { p->~T(); }	// { dg-error "incomplete" }
> +};
> +
> +struct C: B<A> { };


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]