procyberian/gcc
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

invoke.texi (Wsuggest-final-types, [...]): Document.

Browse source 
* doc/invoke.texi (Wsuggest-final-types, Wsuggest-final-methods): Document.
	* ipa-devirt.c: Include hash-map.h
	(struct polymorphic_call_target_d): Add type_warning and decl_warning.
	(clear_speculation): Break out of ...
	(get_class_context): ... here; speed up handling obviously useless
	speculations.
	(odr_type_warn_count, decl_warn_count): New structures.
	(final_warning_record): New structure.
	(final_warning_records): New static variable.
	(possible_polymorphic_call_targets): Cleanup handling of speculative info;
	do not build speculation when user do not care; record info about warnings
	when asked for.
	(add_decl_warning): New function.
	(type_warning_cmp): New function.
	(decl_warning_cmp): New function.
	(ipa_devirt): Handle -Wsuggest-final-methods and -Wsuggest-final-types.
	(gate): Enable pass when warnings are requested.
	* common.opt (Wsuggest-final-types, Wsuggest-final-methods): New options.

	* g++.dg/warn/Wsuggest-final.C: New testcase.
	* g++.dg/ipa/devirt-34.C: Fix.

From-SVN: r213518
This commit is contained in:
Jan Hubicka 2014-08-02 17:13:41 +02:00 • committed by Jan Hubicka
parent b787e7a2c2
commit 91bc34a94d
8 changed files with 327 additions and 46 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

21
gcc/ChangeLog
View file

@ -1,3 +1,24 @@
2014-08-01 Jan Hubicka <hubicka@ucw.cz>
* doc/invoke.texi (Wsuggest-final-types, Wsuggest-final-methods): Document.
* ipa-devirt.c: Include hash-map.h
(struct polymorphic_call_target_d): Add type_warning and decl_warning.
(clear_speculation): Break out of ...
(get_class_context): ... here; speed up handling obviously useless
speculations.
(odr_type_warn_count, decl_warn_count): New structures.
(final_warning_record): New structure.
(final_warning_records): New static variable.
(possible_polymorphic_call_targets): Cleanup handling of speculative info;
do not build speculation when user do not care; record info about warnings
when asked for.
(add_decl_warning): New function.
(type_warning_cmp): New function.
(decl_warning_cmp): New function.
(ipa_devirt): Handle -Wsuggest-final-methods and -Wsuggest-final-types.
(gate): Enable pass when warnings are requested.
* common.opt (Wsuggest-final-types, Wsuggest-final-methods): New options.
2014-08-02 Trevor Saunders <tsaunders@mozilla.com>
* hash-map.h (default_hashmap_traits::mark_key_deleted):

8
gcc/common.opt
View file

@ -651,6 +651,14 @@ Wsuggest-attribute=noreturn
Common Var(warn_suggest_attribute_noreturn) Warning
Warn about functions which might be candidates for __attribute__((noreturn))
Wsuggest-final-types
Common Var(warn_suggest_final_types) Warning
Warn about C++ polymorphic types where adding final keyword would improve code quality
Wsuggest-final-methods
Common Var(warn_suggest_final_methods) Warning
Warn about C++ virtual methods where adding final keyword would improve code quality
Wsystem-headers
Common Var(warn_system_headers) Warning
Do not suppress warnings from system headers

22
gcc/doc/invoke.texi
View file

@ -271,6 +271,7 @@ Objective-C and Objective-C++ Dialects}.
-Wstack-protector -Wstack-usage=@var{len} -Wstrict-aliasing @gol
-Wstrict-aliasing=n @gol -Wstrict-overflow -Wstrict-overflow=@var{n} @gol
-Wsuggest-attribute=@r{[}pure@r{|}const@r{|}noreturn@r{|}format@r{]} @gol
-Wsuggest-final-types @gol -Wsuggest-final-methods @gol
-Wmissing-format-attribute @gol
-Wswitch -Wswitch-default -Wswitch-enum -Wswitch-bool -Wsync-nand @gol
-Wsystem-headers -Wtrampolines -Wtrigraphs -Wtype-limits -Wundef @gol
@ -4193,6 +4194,25 @@ case, and some functions for which @code{format} attributes are
appropriate may not be detected.
@end table
@item -Wsuggest-final-types
@opindex Wno-suggest-final-types
@opindex Wsuggest-final-types
Warn about types with virtual methods where code quality would be improved
if the type was declared with C++11 final specifier, or, if possible,
declared in anonymous namespace. This allows GCC to devritualize more aggressively
the polymorphic calls. This warning is more effective with link time optimization,
where the information about the class hiearchy graph is more complete.
@item -Wsuggest-final-methods
@opindex Wno-suggest-final-methods
@opindex Wsuggest-final-methods
Warn about virtual methods where code quality would be improved if the method
was declared with C++11 final specifier, or, if possible, its type was declared
in the anonymous namespace or with final specifier. This warning is more
effective with link time optimization, where the information about the class
hiearchy graph is more complete. It is recommended to first consider suggestins
of @option{-Wsuggest-final-types} and then rebuild with new annotations.
@item -Warray-bounds
@opindex Wno-array-bounds
@opindex Warray-bounds
@ -9622,7 +9642,7 @@ before applying @option{--param inline-unit-growth}. The default is 10000.
@item inline-unit-growth
Specifies maximal overall growth of the compilation unit caused by inlining.
The default value is 30 which limits unit growth to 1.3 times the original
size. Cold functions (either marked cold via an attribibute or by profile
size. Cold functions (either marked cold via an attribute or by profile
feedback) are not accounted into the unit size.
@item ipcp-unit-growth

288
gcc/ipa-devirt.c
View file

@ -133,6 +133,7 @@ along with GCC; see the file COPYING3. If not see
#include "dbgcnt.h"
#include "stor-layout.h"
#include "intl.h"
#include "hash-map.h"
static bool odr_types_equivalent_p (tree, tree, bool, bool *,
hash_set<tree> *);
@ -1616,6 +1617,8 @@ struct polymorphic_call_target_d
vec <cgraph_node *> targets;
int speculative_targets;
bool complete;
int type_warning;
tree decl_warning;
};
/* Polymorphic call target cache helpers. */
@ -1734,6 +1737,16 @@ contains_polymorphic_type_p (const_tree type)
return false;
}
/* Clear speculative info from CONTEXT. */
static void
clear_speculation (ipa_polymorphic_call_context *context)
{
context->speculative_outer_type = NULL;
context->speculative_offset = 0;
context->speculative_maybe_derived_type = false;
}
/* CONTEXT->OUTER_TYPE is a type of memory object where object of EXPECTED_TYPE
is contained at CONTEXT->OFFSET. Walk the memory representation of
CONTEXT->OUTER_TYPE and find the outermost class type that match
@ -1769,6 +1782,16 @@ get_class_context (ipa_polymorphic_call_context *context,
type = context->outer_type = expected_type;
context->offset = offset = 0;
}
if (context->speculative_outer_type == context->outer_type
&& (!context->maybe_derived_type
|| context->speculative_maybe_derived_type))
{
context->speculative_outer_type = NULL;
context->speculative_offset = 0;
context->speculative_maybe_derived_type = false;
}
/* See if speculative type seem to be derrived from outer_type.
Then speculation is valid only if it really is a derivate and derived types
are allowed.
@ -1785,11 +1808,7 @@ get_class_context (ipa_polymorphic_call_context *context,
context->outer_type))
speculation_valid = context->maybe_derived_type;
else
{
context->speculative_outer_type = NULL;
context->speculative_offset = 0;
context->speculative_maybe_derived_type = false;
}
clear_speculation (context);
/* Find the sub-object the constant actually refers to and mark whether it is
an artificial one (as opposed to a user-defined one).
@ -1820,10 +1839,7 @@ get_class_context (ipa_polymorphic_call_context *context,
context->outer_type)
&& (context->maybe_derived_type
== context->speculative_maybe_derived_type)))
{
context->speculative_outer_type = NULL;
context->speculative_offset = 0;
}
clear_speculation (context);
return true;
}
else
@ -1841,8 +1857,7 @@ get_class_context (ipa_polymorphic_call_context *context,
if (!speculation_valid
|| !context->maybe_derived_type)
{
context->speculative_outer_type = NULL;
context->speculative_offset = 0;
clear_speculation (context);
return true;
}
/* Otherwise look into speculation now. */
@ -1925,9 +1940,7 @@ get_class_context (ipa_polymorphic_call_context *context,
/* If we failed to find subtype we look for, give up and fall back to the
most generic query. */
give_up:
context->speculative_outer_type = NULL;
context->speculative_offset = 0;
context->speculative_maybe_derived_type = false;
clear_speculation (context);
if (valid)
return true;
context->outer_type = expected_type;
@ -2502,6 +2515,30 @@ devirt_variable_node_removal_hook (varpool_node *n,
free_polymorphic_call_targets_hash ();
}
/* Record about how many calls would benefit from given type to be final. */
struct odr_type_warn_count
{
int count;
gcov_type dyn_count;
};
/* Record about how many calls would benefit from given method to be final. */
struct decl_warn_count
{
tree decl;
int count;
gcov_type dyn_count;
};
/* Information about type and decl warnings. */
struct final_warning_record
{
gcov_type dyn_count;
vec<odr_type_warn_count> type_warnings;
hash_map<tree, decl_warn_count> decl_warnings;
};
struct final_warning_record *final_warning_records;
/* Return vector containing possible targets of polymorphic call of type
OTR_TYPE caling method OTR_TOKEN within type of OTR_OUTER_TYPE and OFFSET.
If INCLUDE_BASES is true, walk also base types of OUTER_TYPES containig
@ -2573,6 +2610,10 @@ possible_polymorphic_call_targets (tree otr_type,
return nodes;
}
/* Do not bother to compute speculative info when user do not asks for it. */
if (!speculative_targetsp || !context.speculative_outer_type)
clear_speculation (&context);
type = get_odr_type (otr_type, true);
/* Recording type variants would wast results cache. */
@ -2639,6 +2680,19 @@ possible_polymorphic_call_targets (tree otr_type,
*completep = (*slot)->complete;
if (speculative_targetsp)
*speculative_targetsp = (*slot)->speculative_targets;
if ((*slot)->type_warning && final_warning_records)
{
final_warning_records->type_warnings[(*slot)->type_warning - 1].count++;
final_warning_records->type_warnings[(*slot)->type_warning - 1].dyn_count
+= final_warning_records->dyn_count;
}
if ((*slot)->decl_warning && final_warning_records)
{
struct decl_warn_count *c =
final_warning_records->decl_warnings.get ((*slot)->decl_warning);
c->count++;
c->dyn_count += final_warning_records->dyn_count;
}
return (*slot)->targets;
}
@ -2657,9 +2711,13 @@ possible_polymorphic_call_targets (tree otr_type,
hash_set<tree> inserted;
hash_set<tree> matched_vtables;
/* First insert targets we speculatively identified as likely. */
if (context.speculative_outer_type)
{
odr_type speculative_outer_type;
bool speculation_complete = true;
/* First insert target from type itself and check if it may have derived types. */
speculative_outer_type = get_odr_type (context.speculative_outer_type, true);
if (TYPE_FINAL_P (speculative_outer_type->type))
context.speculative_maybe_derived_type = false;
@ -2671,35 +2729,27 @@ possible_polymorphic_call_targets (tree otr_type,
else
target = NULL;
if (target)
{
/* In the case we get complete method, we don't need
to walk derivations. */
if (DECL_FINAL_P (target))
context.speculative_maybe_derived_type = false;
}
/* In the case we get complete method, we don't need
to walk derivations. */
if (target && DECL_FINAL_P (target))
context.speculative_maybe_derived_type = false;
if (type_possibly_instantiated_p (speculative_outer_type->type))
maybe_record_node (nodes, target, &inserted, can_refer, &complete);
maybe_record_node (nodes, target, &inserted, can_refer, &speculation_complete);
if (binfo)
matched_vtables.add (BINFO_VTABLE (binfo));
/* Next walk recursively all derived types. */
if (context.speculative_maybe_derived_type)
{
/* For anonymous namespace types we can attempt to build full type.
All derivations must be in this unit (unless we see partial unit). */
if (!type->all_derivations_known)
complete = false;
for (i = 0; i < speculative_outer_type->derived_types.length(); i++)
possible_polymorphic_call_targets_1 (nodes, &inserted,
&matched_vtables,
otr_type,
speculative_outer_type->derived_types[i],
otr_token, speculative_outer_type->type,
context.speculative_offset, &complete,
bases_to_consider,
false);
}
/* Finally walk bases, if asked to. */
for (i = 0; i < speculative_outer_type->derived_types.length(); i++)
possible_polymorphic_call_targets_1 (nodes, &inserted,
&matched_vtables,
otr_type,
speculative_outer_type->derived_types[i],
otr_token, speculative_outer_type->type,
context.speculative_offset,
&speculation_complete,
bases_to_consider,
false);
(*slot)->speculative_targets = nodes.length();
}
@ -2743,10 +2793,6 @@ possible_polymorphic_call_targets (tree otr_type,
/* Next walk recursively all derived types. */
if (context.maybe_derived_type)
{
/* For anonymous namespace types we can attempt to build full type.
All derivations must be in this unit (unless we see partial unit). */
if (!type->all_derivations_known)
complete = false;
for (i = 0; i < outer_type->derived_types.length(); i++)
possible_polymorphic_call_targets_1 (nodes, &inserted,
&matched_vtables,
@ -2756,6 +2802,51 @@ possible_polymorphic_call_targets (tree otr_type,
context.offset, &complete,
bases_to_consider,
context.maybe_in_construction);
if (!outer_type->all_derivations_known)
{
if (final_warning_records)
{
if (complete
&& nodes.length () == 1
&& warn_suggest_final_types
&& !outer_type->derived_types.length ())
{
if (outer_type->id >= (int)final_warning_records->type_warnings.length ())
final_warning_records->type_warnings.safe_grow_cleared
(odr_types.length ());
final_warning_records->type_warnings[outer_type->id].count++;
final_warning_records->type_warnings[outer_type->id].dyn_count
+= final_warning_records->dyn_count;
(*slot)->type_warning = outer_type->id + 1;
}
if (complete
&& warn_suggest_final_methods
&& nodes.length () == 1
&& types_same_for_odr (DECL_CONTEXT (nodes[0]->decl),
outer_type->type))
{
bool existed;
struct decl_warn_count &c =
final_warning_records->decl_warnings.get_or_insert
(nodes[0]->decl, &existed);
if (existed)
{
c.count++;
c.dyn_count += final_warning_records->dyn_count;
}
else
{
c.count = 1;
c.dyn_count = final_warning_records->dyn_count;
c.decl = nodes[0]->decl;
}
(*slot)->decl_warning = nodes[0]->decl;
}
}
complete = false;
}
}
/* Finally walk bases, if asked to. */
@ -2796,6 +2887,14 @@ possible_polymorphic_call_targets (tree otr_type,
return nodes;
}
bool
add_decl_warning (const tree &key ATTRIBUTE_UNUSED, const decl_warn_count &value,
vec<const decl_warn_count*> *vec)
{
vec->safe_push (&value);
return true;
}
/* Dump all possible targets of a polymorphic call. */
void
@ -2946,6 +3045,38 @@ likely_target_p (struct cgraph_node *n)
return true;
}
/* Compare type warning records P1 and P2 and chose one with larger count;
helper for qsort. */
int
type_warning_cmp (const void *p1, const void *p2)
{
const odr_type_warn_count *t1 = (const odr_type_warn_count *)p1;
const odr_type_warn_count *t2 = (const odr_type_warn_count *)p2;
if (t1->dyn_count < t2->dyn_count)
return 1;
if (t1->dyn_count > t2->dyn_count)
return -1;
return t2->count - t1->count;
}
/* Compare decl warning records P1 and P2 and chose one with larger count;
helper for qsort. */
int
decl_warning_cmp (const void *p1, const void *p2)
{
const decl_warn_count *t1 = *(const decl_warn_count * const *)p1;
const decl_warn_count *t2 = *(const decl_warn_count * const *)p2;
if (t1->dyn_count < t2->dyn_count)
return 1;
if (t1->dyn_count > t2->dyn_count)
return -1;
return t2->count - t1->count;
}
/* The ipa-devirt pass.
When polymorphic call has only one likely target in the unit,
turn it into speculative call. */
@ -2961,6 +3092,19 @@ ipa_devirt (void)
int nmultiple = 0, noverwritable = 0, ndevirtualized = 0, nnotdefined = 0;
int nwrong = 0, nok = 0, nexternal = 0, nartificial = 0;
/* We can output -Wsuggest-final-methods and -Wsuggest-final-types warnings.
This is implemented by setting up final_warning_records that are updated
by get_polymorphic_call_targets.
We need to clear cache in this case to trigger recomputation of all
entries. */
if (warn_suggest_final_methods || warn_suggest_final_types)
{
final_warning_records = new (final_warning_record);
final_warning_records->type_warnings = vNULL;
final_warning_records->type_warnings.safe_grow_cleared (odr_types.length ());
free_polymorphic_call_targets_hash ();
}
FOR_EACH_DEFINED_FUNCTION (n)
{
bool update = false;
@ -2974,6 +3118,10 @@ ipa_devirt (void)
void *cache_token;
bool final;
int speculative_targets;
if (final_warning_records)
final_warning_records->dyn_count = e->count;
vec <cgraph_node *>targets
= possible_polymorphic_call_targets
(e, &final, &cache_token, &speculative_targets);
@ -2985,6 +3133,9 @@ ipa_devirt (void)
npolymorphic++;
if (!flag_devirtualize_speculatively)
continue;
if (!cgraph_maybe_hot_edge_p (e))
{
if (dump_file)
@ -3114,6 +3265,55 @@ ipa_devirt (void)
if (update)
inline_update_overall_summary (n);
}
if (warn_suggest_final_methods || warn_suggest_final_types)
{
if (warn_suggest_final_types)
{
final_warning_records->type_warnings.qsort (type_warning_cmp);
for (unsigned int i = 0;
i < final_warning_records->type_warnings.length (); i++)
if (final_warning_records->type_warnings[i].count)
{
odr_type type = odr_types[i];
warning_at (DECL_SOURCE_LOCATION (TYPE_NAME (type->type)),
OPT_Wsuggest_final_types,
"Declaring type %qD final "
"would enable devirtualization of %i calls",
type->type,
final_warning_records->type_warnings[i].count);
}
}
if (warn_suggest_final_methods)
{
vec<const decl_warn_count*> decl_warnings_vec = vNULL;
final_warning_records->decl_warnings.traverse
<vec<const decl_warn_count *> *, add_decl_warning> (&decl_warnings_vec);
decl_warnings_vec.qsort (decl_warning_cmp);
for (unsigned int i = 0; i < decl_warnings_vec.length (); i++)
{
tree decl = decl_warnings_vec[i]->decl;
int count = decl_warnings_vec[i]->count;
if (DECL_CXX_DESTRUCTOR_P (decl))
warning_at (DECL_SOURCE_LOCATION (decl),
OPT_Wsuggest_final_methods,
"Declaring virtual destructor of %qD final "
"would enable devirtualization of %i calls",
DECL_CONTEXT (decl), count);
else
warning_at (DECL_SOURCE_LOCATION (decl),
OPT_Wsuggest_final_methods,
"Declaring method %qD final "
"would enable devirtualization of %i calls",
decl, count);
}
}
delete (final_warning_records);
final_warning_records = 0;
}
if (dump_file)
fprintf (dump_file,
@ -3163,7 +3363,9 @@ public:
virtual bool gate (function *)
{
return (flag_devirtualize
&& flag_devirtualize_speculatively
&& (flag_devirtualize_speculatively
|| (warn_suggest_final_methods
|| warn_suggest_final_types))
&& optimize);
}

5
gcc/testsuite/ChangeLog
View file

@ -1,3 +1,8 @@
2014-08-01 Jan Hubicka <hubicka@ucw.cz>
* g++.dg/warn/Wsuggest-final.C: New testcase.
* g++.dg/ipa/devirt-34.C: Fix.
2014-08-02 Marek Polacek <polacek@redhat.com>
PR c/59855

3
gcc/testsuite/g++.dg/ipa/devirt-34.C
View file

@ -2,6 +2,9 @@
/* { dg-options "-O2 -fdump-ipa-devirt" } */
struct A {virtual int t(){return 42;}};
struct B:A {virtual int t(){return 1;}};
struct A aa;
struct B bb;
int
t(struct B *b)
{

14
gcc/testsuite/g++.dg/warn/Wsuggest-final.C Normal file
View file

@ -0,0 +1,14 @@
// { dg-do compile }
// { dg-options "-O2 -Wsuggest-final-types -Wsuggest-final-methods" }
struct A { // { dg-warning "final would enable devirtualization of 4 calls" }
virtual void a() {} // { dg-warning "final would enable devirtualization of 2 calls" }
virtual void b() {} // { dg-warning "final would enable devirtualization of 2 calls" }
};
void
t(struct A *a)
{
a->a();
a->a();
a->b();
a->b();
}

12
gcc/varpool.c
View file

@ -340,8 +340,16 @@ varpool_node::ctor_useable_for_folding_p (void)
/* Variables declared 'const' without an initializer
have zero as the initializer if they may not be
overridden at link or run time. */
if (!DECL_INITIAL (real_node->decl)
overridden at link or run time.
It is actually requirement for C++ compiler to optimize const variables
consistently. As a GNU extension, do not enfore this rule for user defined
weak variables, so we support interposition on:
static const int dummy = 0;
extern const int foo __attribute__((__weak__, __alias__("dummy")));
*/
if ((!DECL_INITIAL (real_node->decl)
|| (DECL_WEAK (decl) && !DECL_COMDAT (decl)))
&& (DECL_EXTERNAL (decl) || decl_replaceable_p (decl)))
return false;