Re: [PATCH 04/16] Add test-cfg.c to unittests

[Jeff Law <law at redhat dot com>]

On 10/27/2015 01:49 PM, David Malcolm wrote:
> gcc/testsuite/ChangeLog:
> 	* unittests/test-cfg.c: New file.
> ---
>   gcc/testsuite/unittests/test-cfg.c | 319 +++++++++++++++++++++++++++++++++++++
>   1 file changed, 319 insertions(+)
>   create mode 100644 gcc/testsuite/unittests/test-cfg.c
>
> diff --git a/gcc/testsuite/unittests/test-cfg.c b/gcc/testsuite/unittests/test-cfg.c
>
> +/* These tests directly create CFGs.
> +   Compare with the static fns within tree-cfg.c:
> +     - build_gimple_cfg
> +     - make_blocks: calls create_basic_block (seq, bb);
> +     - make_edges.   */
> +
> +/* Verify a simple cfg of the form:
> +     ENTRY -> A -> B -> C -> EXIT.  */
> +TEST_F (cfg_test, linear_chain)
> +{
> +  gimple_register_cfg_hooks ();
> +
> +  tree fndecl = push_fndecl ("cfg_test_linear_chain");
> +  function *fun = DECL_STRUCT_FUNCTION (fndecl);
> +  EXPECT_TRUE (fun != NULL);
> +
> +  EXPECT_EQ (2, n_basic_blocks_for_fn (fun));
> +  EXPECT_EQ (0, n_edges_for_fn (fun));
Aren't those 3 redundant with the tests done by push_fndecl?


> +
> +/* Verify a simple CFG of the form:
> +     ENTRY
> +       |
> +       A
> +      / \
> +     /t  \f
> +    B     C
> +     \   /
> +      \ /
> +       D
> +       |
> +      EXIT.  */
> +TEST_F (cfg_test, diamond)
> +{
> +  gimple_register_cfg_hooks ();
> +
> +  tree fndecl = push_fndecl ("cfg_test_diamond");
> +  function *fun = DECL_STRUCT_FUNCTION (fndecl);
> +  EXPECT_TRUE (fun != NULL);
> +
> +  EXPECT_EQ (2, n_basic_blocks_for_fn (fun));
> +  EXPECT_EQ (0, n_edges_for_fn (fun));
Similarly here.


> +}  /* anon namespace.  */
> +
> +/* TODO: test the dominator/postdominator logic with various graphs/nodes:
> +   - loop
> +   - nested loops
> +   - switch statement (a block with many out-edges)
> +   - something that jumps to itself
> +   - etc  */
> +
> +/* TODO: add tests for loop-detection here?  */
I wonder what would happen if you called any of the internal cfg 
verification routines?!?

/me ducks.


Unless there's a good reason, drop the presumably redundant tests and 
this is OK.  Save preapprovald for these changes as the bitmap patch.

jeff