diff options
| -rw-r--r-- | gcc/ChangeLog | 21 | ||||
| -rw-r--r-- | gcc/common.opt | 8 | ||||
| -rw-r--r-- | gcc/doc/invoke.texi | 22 | ||||
| -rw-r--r-- | gcc/ipa-devirt.c | 288 | ||||
| -rw-r--r-- | gcc/testsuite/ChangeLog | 5 | ||||
| -rw-r--r-- | gcc/testsuite/g++.dg/ipa/devirt-34.C | 3 | ||||
| -rw-r--r-- | gcc/testsuite/g++.dg/warn/Wsuggest-final.C | 14 | ||||
| -rw-r--r-- | gcc/varpool.c | 12 |
8 files changed, 327 insertions, 46 deletions
diff --git a/gcc/ChangeLog b/gcc/ChangeLog index 7302bc8befc..832c89b1684 100644 --- a/gcc/ChangeLog +++ b/gcc/ChangeLog @@ -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): diff --git a/gcc/common.opt b/gcc/common.opt index 40c8b3c9bb9..0c4f86bb14c 100644 --- a/gcc/common.opt +++ b/gcc/common.opt @@ -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 diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi index 1eefb69826b..4f327df69eb 100644 --- a/gcc/doc/invoke.texi +++ b/gcc/doc/invoke.texi @@ -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 diff --git a/gcc/ipa-devirt.c b/gcc/ipa-devirt.c index 5484ccde4a6..ca74d3bb4fd 100644 --- a/gcc/ipa-devirt.c +++ b/gcc/ipa-devirt.c @@ -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); } diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog index 13b63233750..aab609706f9 100644 --- a/gcc/testsuite/ChangeLog +++ b/gcc/testsuite/ChangeLog @@ -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 diff --git a/gcc/testsuite/g++.dg/ipa/devirt-34.C b/gcc/testsuite/g++.dg/ipa/devirt-34.C index 258a2aded46..5d56e1e0c8b 100644 --- a/gcc/testsuite/g++.dg/ipa/devirt-34.C +++ b/gcc/testsuite/g++.dg/ipa/devirt-34.C @@ -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) { diff --git a/gcc/testsuite/g++.dg/warn/Wsuggest-final.C b/gcc/testsuite/g++.dg/warn/Wsuggest-final.C new file mode 100644 index 00000000000..5371063559d --- /dev/null +++ b/gcc/testsuite/g++.dg/warn/Wsuggest-final.C @@ -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(); +} diff --git a/gcc/varpool.c b/gcc/varpool.c index 41b83d7599c..8350adb2d87 100644 --- a/gcc/varpool.c +++ b/gcc/varpool.c @@ -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; |