[1/3] Come up with selftests for predict.c.

gcc/ChangeLog:

2017-05-26  Martin Liska  <mliska@suse.cz>

	* predict.c (struct branch_predictor): New struct.
	(test_prediction_value_range): New test.
	(predict_tests_c_tests): New function.
	* selftest-run-tests.c (selftest::run_tests): Run the function.
	* selftest.h: Declare new tests.
---
 gcc/predict.c            | 39 +++++++++++++++++++++++++++++++++++++++
 gcc/selftest-run-tests.c |  1 +
 gcc/selftest.h           |  1 +
 3 files changed, 41 insertions(+)

On Tue, 2017-06-06 at 10:55 +0200, marxin wrote:

Some minor nits below.

> gcc/ChangeLog:
> 
> 2017-05-26  Martin Liska  <mliska@suse.cz>
> 
> 	* predict.c (struct branch_predictor): New struct.
> 	(test_prediction_value_range): New test.
> 	(predict_tests_c_tests): New function.
> 	* selftest-run-tests.c (selftest::run_tests): Run the function.
> 	* selftest.h: Declare new tests.
> ---
>  gcc/predict.c            | 39
> +++++++++++++++++++++++++++++++++++++++
>  gcc/selftest-run-tests.c |  1 +
>  gcc/selftest.h           |  1 +
>  3 files changed, 41 insertions(+)
> 
> diff --git a/gcc/predict.c b/gcc/predict.c
> index ac35fa41129..ea445e94e46 100644
> --- a/gcc/predict.c
> +++ b/gcc/predict.c
> @@ -57,6 +57,7 @@ along with GCC; see the file COPYING3.  If not see
>  #include "tree-scalar-evolution.h"
>  #include "ipa-utils.h"
>  #include "gimple-pretty-print.h"
> +#include "selftest.h"
>  
>  /* Enum with reasons why a predictor is ignored.  */
>  
> @@ -3803,3 +3804,41 @@ force_edge_cold (edge e, bool impossible)
>  		 impossible ? "impossible" : "cold");
>      }
>  }
> +
> +#if CHECKING_P
> +
> +namespace selftest {
> +
> +/* Test that value range of predictor values defined in predict.def
> is
> +   within range [51, 100].  */
> +
> +struct branch_predictor
> +{
> +  const char *name;
> +  unsigned probability;
> +};
> +
> +#define DEF_PREDICTOR(ENUM, NAME, HITRATE, FLAGS) { NAME, HITRATE },
> +
> +static void
> +test_prediction_value_range ()
> +{
> +  branch_predictor predictors[] = {
> +#include "predict.def"
> +    {NULL, -1}
> +  };
> +
> +  for (unsigned i = 0; predictors[i].name != NULL; i++)
> +    ASSERT_TRUE (100 * predictors[i].probability / REG_BR_PROB_BASE
> > 50);

Minor nit: should there be a #undef DEF_PREDICTOR after the #include?

Minor nits: the comment says it verifies that things are in the range
51, 100.  Should it be >= 51 rather than > 50?  Should there be a test
that it's <= 100?  (I'm not familiar with the predict code, I just
noticed this when reading the comment).

> +}
> +
> +/* Run all of the selfests within this file.  */
> +
> +void
> +predict_tests_c_tests ()
> +{

Minor nit: to follow the pattern used in the other selftests, we've
been naming these "run all selftests within this file" functions after
the filename.  Given this is in predict.c, this should be
"predict_c_tests ()" to follow the pattern.

> +  test_prediction_value_range ();
> +}
> +
> +} // namespace selftest
> +#endif /* CHECKING_P.  */
> diff --git a/gcc/selftest-run-tests.c b/gcc/selftest-run-tests.c
> index f62bc72b072..af511a47681 100644
> --- a/gcc/selftest-run-tests.c
> +++ b/gcc/selftest-run-tests.c
> @@ -92,6 +92,7 @@ selftest::run_tests ()
>      targetm.run_target_selftests ();
>  
>    store_merging_c_tests ();
> +  predict_tests_c_tests ();

...and here.


>    /* Run any lang-specific selftests.  */
>    lang_hooks.run_lang_selftests ();
> diff --git a/gcc/selftest.h b/gcc/selftest.h
> index dad53e9fe09..e84b309359d 100644
> --- a/gcc/selftest.h
> +++ b/gcc/selftest.h
> @@ -196,6 +196,7 @@ extern void tree_c_tests ();
>  extern void tree_cfg_c_tests ();
>  extern void vec_c_tests ();
>  extern void wide_int_cc_tests ();
> +extern void predict_tests_c_tests ();

(etc)
 
>  extern int num_passes;

Thanks; hope this is constructive.
Dave