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: [PATCH] Make -Wcoverage-mismatch behavior sane.


cc (+): hubicka@ucw.cz (including this since it seems to be a more
active email address for Jan)

On Wed, Apr 28, 2010 at 7:24 PM, Neil Vachharajani <nvachhar@google.com> wrote:
> By default, coverage mismatches are errors. ?Unlike every other -W
> option, -Wcoverage-mismatch demotes the error to a warning (rather
> than simply enabling the warning). ?Unfortunately, when compiling with
> -Werror, this warning gets *re-promoted* to an error. ?There is a
> difference between the original error (i.e., when compiling w/o
> -Wcoverage-mismatch) and this new error (created by promoting a
> warning to an error). ?-Wno-error=coverage-mismatch will prevent the
> warning from being promoted to an error. ?So you need both options
> (-Wcoverage-mismatch and -Wno-error=coverage-mismatch) to guarantee
> that coverage mismatch's are only reported as warnings and not errors.
>
> This patch changes the behavior so that -Wcoverage-mismatch enables
> the coverage mismatch warnings. ?It is on by default and can be
> negated (unlike before). ?To maintain the current behavior that
> coverage mismatches are reported as errors, by default
> -Werror=coverage-mismatch is also enabled. ?The warning can be
> disabled with -Wno-coverage-mismatch and the error can be disabled
> with -Wno-error=coverage-mismatch.
> Bootstrapped and regtested on x86_64. ?Ok for trunk?
>
> 2010-04-28 ?Neil Vachharajani <nvachhar@google.com>
>
> ?? ? ? ?* doc/invoke.texi (-Wcoverage-mismatch): Updated documentation as
> ?? ? ? ?per new semantics.
> ?? ? ? ?* opts.c (decode_options): Enable -Werror=coverage-mismatch.
> ?? ? ? ?* coverage.c (get_coverage_counts): Always emit a warning. ?Adjust
> ?? ? ? ?conditions for printing notes.
> ?? ? ? ?* common.opt (-Wcoverage-mismatch): Allow negative, default to
> ?? ? ? ?true, update documentation.
> ?? ? ? ?* Makefile.in (coverage.o): Add dependence on DIAGNOSTIC_H.
>
> diff --git a/gcc/gcc/Makefile.in b/gcc/gcc/Makefile.in
> index ca68570..884d254 100644
> --- a/gcc/gcc/Makefile.in
> +++ b/gcc/gcc/Makefile.in
> @@ -2942,7 +2942,8 @@ ipa-struct-reorg.o: ipa-struct-reorg.c
> ipa-struct-reorg.h $(CONFIG_H) $(SYSTEM_H
> ?coverage.o : coverage.c $(GCOV_IO_H) $(CONFIG_H) $(SYSTEM_H) coretypes.h \
> ?? ?$(TM_H) $(RTL_H) $(TREE_H) $(FLAGS_H) output.h $(REGS_H) $(EXPR_H) \
> ?? ?$(FUNCTION_H) $(TOPLEV_H) $(GGC_H) langhooks.h $(COVERAGE_H) gt-coverage.h \
> - ? $(HASHTAB_H) tree-iterator.h $(CGRAPH_H) $(TREE_PASS_H) gcov-io.c $(TM_P_H)
> + ? $(HASHTAB_H) tree-iterator.h $(CGRAPH_H) $(TREE_PASS_H) gcov-io.c
> $(TM_P_H) \
> + ? $(DIAGNOSTIC_H)
> ?cselib.o : cselib.c $(CONFIG_H) $(SYSTEM_H) coretypes.h $(TM_H) $(RTL_H) \
> ?? ?$(REGS_H) hard-reg-set.h $(FLAGS_H) $(REAL_H) insn-config.h $(RECOG_H) \
> ?? ?$(EMIT_RTL_H) $(TOPLEV_H) output.h $(FUNCTION_H) $(TREE_PASS_H) \
> diff --git a/gcc/gcc/common.opt b/gcc/gcc/common.opt
> index e62e3d5..806bd1f 100644
> --- a/gcc/gcc/common.opt
> +++ b/gcc/gcc/common.opt
> @@ -249,8 +249,8 @@ Common Var(warn_unused_variable) Init(-1) Warning
> ?Warn when a variable is unused
>
> ?Wcoverage-mismatch
> -Common RejectNegative Var(warn_coverage_mismatch) Warning
> -Warn instead of error in case profiles in -fprofile-use do not match
> +Common Var(warn_coverage_mismatch) Init(1) Warning
> +Warn in case profiles in -fprofile-use do not match
>
> ?aux-info
> ?Common Separate
> diff --git a/gcc/gcc/coverage.c b/gcc/gcc/coverage.c
> index e04d22b..1dd89d1 100644
> --- a/gcc/gcc/coverage.c
> +++ b/gcc/gcc/coverage.c
> @@ -46,6 +46,7 @@ along with GCC; see the file COPYING3. ?If not see
> ?#include "tree-iterator.h"
> ?#include "cgraph.h"
> ?#include "tree-pass.h"
> +#include "diagnostic.h"
>
> ?#include "gcov-io.c"
>
> @@ -360,31 +361,30 @@ get_coverage_counts (unsigned counter, unsigned expected,
> ?? ? ? tree id = DECL_ASSEMBLER_NAME (current_function_decl);
>
> ?? ? ? if (warn_coverage_mismatch)
> - warning (OPT_Wcoverage_mismatch, "coverage mismatch for function "
> - "%qE while reading counter %qs", id, ctr_names[counter]);
> - ? ? ?else
> - error ("coverage mismatch for function %qE while reading counter %qs",
> - ? ? ? id, ctr_names[counter]);
> -
> - ? ? ?if (!inhibit_warnings)
> ? {
> - ?if (entry->checksum != checksum)
> - ? ?inform (input_location, "checksum is %x instead of %x",
> entry->checksum, checksum);
> - ?else
> - ? ?inform (input_location, "number of counters is %d instead of %d",
> - ? ?entry->summary.num, expected);
> - }
> + ?warning (OPT_Wcoverage_mismatch, "coverage mismatch for function "
> + ? "%qE while reading counter %qs", id, ctr_names[counter]);
>
> - ? ? ?if (warn_coverage_mismatch
> - ?&& !inhibit_warnings
> - ?&& !warned++)
> - {
> - ?inform (input_location, "coverage mismatch ignored due to
> -Wcoverage-mismatch");
> - ?inform (input_location, flag_guess_branch_prob
> - ?? "execution counts estimated"
> - ?: "execution counts assumed to be zero");
> - ?if (!flag_guess_branch_prob)
> - ? ?inform (input_location, "this can result in poorly optimized code");
> + ?if (!inhibit_warnings)
> + ? ?{
> + ? ? ?if (entry->checksum != checksum)
> + inform (input_location, "checksum is %x instead of %x",
> entry->checksum, checksum);
> + ? ? ?else
> + inform (input_location, "number of counters is %d instead of %d",
> + entry->summary.num, expected);
> + ? ?}
> +
> + ?if (!(errorcount || sorrycount)
> + ? ? ?&& !inhibit_warnings
> + ? ? ?&& !warned++)
> + ? ?{
> + ? ? ?inform (input_location, "coverage mismatch ignored");
> + ? ? ?inform (input_location, flag_guess_branch_prob
> + ? ? ?? "execution counts estimated"
> + ? ? ?: "execution counts assumed to be zero");
> + ? ? ?if (!flag_guess_branch_prob)
> + inform (input_location, "this can result in poorly optimized code");
> + ? ?}
> ? }
>
> ?? ? ? return NULL;
> diff --git a/gcc/gcc/doc/invoke.texi b/gcc/gcc/doc/invoke.texi
> index fefb0ab..492b82b 100644
> --- a/gcc/gcc/doc/invoke.texi
> +++ b/gcc/gcc/doc/invoke.texi
> @@ -2746,12 +2746,13 @@ Warn if feedback profiles do not match when using the
> ?If a source file was changed between @option{-fprofile-gen} and
> ?@option{-fprofile-use}, the files with the profile feedback can fail
> ?to match the source file and GCC can not use the profile feedback
> -information. ?By default, GCC emits an error message in this case.
> -The option @option{-Wcoverage-mismatch} emits a warning instead of an
> -error. ?GCC does not use appropriate feedback profiles, so using this
> -option can result in poorly optimized code. ?This option is useful
> -only in the case of very minor changes such as bug fixes to an
> -existing code-base.
> +information. ?By default, this warning is enabled and is treated as an
> +error. ?@option{-Wno-coverage-mismatch} can be used to disable the
> +warning or @option{-Wno-error=coverage-mismatch} can be used to
> +disable the error. ?Disable the error for this warning can result in
> +poorly optimized code, so disabling the error is useful only in the
> +case of very minor changes such as bug fixes to an existing code-base.
> +Completely disabling the warning is not recommended.
>
> ?@end table
>
> diff --git a/gcc/gcc/opts.c b/gcc/gcc/opts.c
> index 507b502..e5562ab 100644
> --- a/gcc/gcc/opts.c
> +++ b/gcc/gcc/opts.c
> @@ -767,6 +767,11 @@ handle_options (unsigned int argc, const char
> **argv, unsigned int lang_mask)
> ?? ? }
> ?}
>
> +
> +/* Callback function, called when -Werror= enables a warning. ?*/
> +
> +static void (*warning_as_error_callback) (int) = NULL;
> +
> ?/* Parse command line options and set default flag values. ?Do minimal
> ?? ?options processing. ?*/
> ?void
> @@ -946,6 +951,11 @@ decode_options (unsigned int argc, const char **argv)
> ?? else
> ?? ? set_param_value ("min-crossjump-insns", initial_min_crossjump_insns);
>
> + ?/* Enable -Werror=coverage-mismatch by default */
> + ?diagnostic_classify_diagnostic (global_dc, OPT_Wcoverage_mismatch, DK_ERROR);
> + ?if (warning_as_error_callback)
> + ? ?warning_as_error_callback (OPT_Wcoverage_mismatch);
> +
> ?? if (first_time_p)
> ?? ? {
> ?? ? ? /* Initialize whether `char' is signed. ?*/
> @@ -2406,11 +2416,6 @@ set_option (const struct cl_option *option, int
> value, const char *arg)
> ?? ? }
> ?}
>
> -
> -/* Callback function, called when -Werror= enables a warning. ?*/
> -
> -static void (*warning_as_error_callback) (int) = NULL;
> -
> ?/* Register a callback for enable_warning_as_error calls. ?*/
>
> ?void
>



-- 
Neil Vachharajani
Google
650-214-1804


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