On Thu, Mar 2, 2023 at 3:31 AM Xionghu Luo via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
> When spliting edge with self loop, the split edge should be placed just next 
> to
> the edge_in->src, otherwise it may generate different position latch bbs for
> two consecutive self loops.  For details, please refer to:
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93680#c4
>
> Regression tested pass on x86_64-linux-gnu and aarch64-linux-gnu, OK for
> master?
>
> gcc/ChangeLog:
>
>         PR gcov/93680
>         * tree-cfg.cc (split_edge_bb_loc): Return edge_in->src for self loop.
>
> gcc/testsuite/ChangeLog:
>
>         PR gcov/93680
>         * gcc.misc-tests/gcov-pr93680.c: New test.
>
> Signed-off-by: Xionghu Luo <xionghu...@tencent.com>
> ---
>  gcc/testsuite/gcc.misc-tests/gcov-pr93680.c | 24 +++++++++++++++++++++
>  gcc/tree-cfg.cc                             |  2 +-
>  2 files changed, 25 insertions(+), 1 deletion(-)
>  create mode 100644 gcc/testsuite/gcc.misc-tests/gcov-pr93680.c
>
> diff --git a/gcc/testsuite/gcc.misc-tests/gcov-pr93680.c 
> b/gcc/testsuite/gcc.misc-tests/gcov-pr93680.c
> new file mode 100644
> index 00000000000..b2bf9e626fc
> --- /dev/null
> +++ b/gcc/testsuite/gcc.misc-tests/gcov-pr93680.c
> @@ -0,0 +1,24 @@
> +/* { dg-options "-fprofile-arcs -ftest-coverage" } */
> +/* { dg-do run { target native } } */
> +
> +int f(int s, int n)
> +{
> +  int p = 0;
> +
> +  switch (s)
> +  {
> +    case 0: /* count(5) */
> +      do { p++; } while (--n); /* count(5) */
> +      return p; /* count(1) */
> +
> +    case 1: /* count(5) */
> +      do { p++; } while (--n); /* count(5) */
> +      return p; /* count(1) */
> +  }
> +
> +  return 0;
> +}
> +
> +int main() { f(0, 5); f(1, 5); return 0; }
> +
> +/* { dg-final { run-gcov gcov-pr93680.c } } */
> diff --git a/gcc/tree-cfg.cc b/gcc/tree-cfg.cc
> index a9fcc7fd050..6fa1d83d366 100644
> --- a/gcc/tree-cfg.cc
> +++ b/gcc/tree-cfg.cc
> @@ -3009,7 +3009,7 @@ split_edge_bb_loc (edge edge_in)
>    if (dest_prev)
>      {
>        edge e = find_edge (dest_prev, dest);
> -      if (e && !(e->flags & EDGE_COMPLEX))
> +      if ((e && !(e->flags & EDGE_COMPLEX)) || edge_in->src == edge_in->dest)

I think this should eventually apply to all backedge edge_in, correct?
 But of course
we cannot easily test for this here.

Still since this affects ordering in the {next,prev}_bb chain only but not CFG
semantics I wonder how it can affect coverage?  Isn't it only by chance that
this block order survives?

For the case when both edge_in->src has more than one successor and
edge_in->dest has more than one predecessor there isn't any good heuristic
to make printing the blocks in chain order "nice" (well, the backedge
one maybe).

But as said - this order shouldn't have any effect on semantics ...

>         return edge_in->src;
>      }
>    return dest_prev;
> --
> 2.27.0
>

Reply via email to