[google,gcc-4_8] Don't use gcov counter related ssa name as induction variables

Here is the updated patch, which follow UD chain to determine whether
iv.base is defined by __gcovx.xxx[] var. It is a lot simpler than
adding a tree bit.

regression test and previously failed benchmark in piii mode is ok.
Other test is going on.

2014-02-10  Wei Mi  <wmi@google.com>

        * tree-ssa-loop-ivopts.c (defined_by_gcov_counter): New.
        (contains_abnormal_ssa_name_p): Add defined_by_gcov_counter
        check for ssa name.

        * testsuite/gcc.dg/profile-generate-4.c: New.

+/* { dg-final { cleanup-tree-dump "ivopts" } } */

On Mon, Feb 10, 2014 at 11:48 PM, Wei Mi <wmi@google.com> wrote:
> Here is the updated patch, which follow UD chain to determine whether
> iv.base is defined by __gcovx.xxx[] var. It is a lot simpler than
> adding a tree bit.
>
> regression test and previously failed benchmark in piii mode is ok.
> Other test is going on.
>
> 2014-02-10  Wei Mi  <wmi@google.com>
>
>         * tree-ssa-loop-ivopts.c (defined_by_gcov_counter): New.
>         (contains_abnormal_ssa_name_p): Add defined_by_gcov_counter
>         check for ssa name.
>
>         * testsuite/gcc.dg/profile-generate-4.c: New.
>
> Index: tree-ssa-loop-ivopts.c
> ===================================================================
> --- tree-ssa-loop-ivopts.c      (revision 207019)
> +++ tree-ssa-loop-ivopts.c      (working copy)
> @@ -705,6 +705,68 @@ idx_contains_abnormal_ssa_name_p (tree b
>    return !abnormal_ssa_name_p (*index);
>  }
>
> +/* Return true if the use is defined by a gcov counter var.
> +   It is used to check if an iv candidate is generated for
> +   profiling. For profile generated ssa name, we should not
> +   use it as IV because gcov counter may have data-race for
> +   multithread program, it could involve tricky bug to use
> +   such ssa var in IVOPT.
> +

Add the snippets describing how ralloc introduces a second gcov
counter load and asynchronous update of it from another thread leading
to bogus trip count.

Also mention that setting volatile flag on gcov counter accesses may
greatly degrade profile-gen performance.

> +   To limit patterns to be checked, we list the possible cases
> +   here:
> +   Before PRE, the ssa name used to set __gcov counter is as
> +   follows:
> +   for () {
> +     PROF_edge_counter_1 = __gcov.foo[i];
> +     PROF_edge_counter_2 = PROF_edge_counter_1 + 1;
> +     __gcov.foo[i] = PROF_edge_counter_2;
> +   }
> +   If PRE works, the loop may be transformed to:
> +   pretmp_1 = __gcov.foo[i];
> +   for () {
> +     prephitmp_1 = PHI (PROF_edge_counter_2, pretmp_1);
> +     PROF_edge_counter_1 = prephitmp_1;
> +     PROF_edge_counter_2 = PROF_edge_counter_1 + 1;
> +     __gcov.foo[i] = PROF_edge_counter_2;
> +   }
> +   So there are two cases:
> +   case1: If PRE doesn't work, PROF_edge_counter_1 and PROF_edge_counter_2
> +   are neither induction variables candidates. We don't have to worry
> +   about this case.
> +   case2: If PRE works, the iv candidate base of PROF_edge_counter_1 and
> +   PROF_edge_counter_2 are pretmp_1 or pretmp_1 + 1. pretmp_1 is defined
> +   by __gcov var.
> +
> +   So this func only has to check case2. For a ssa name which is an iv
> +   candidate, check its base USE and see if it is defined by __gcov var.
> +   Returning true means the ssa name is generated for profiling.  */
> +
> +bool
> +defined_by_gcov_counter (tree use)
> +{
> +  gimple stmt;
> +  tree rhs, decl;
> +  const char *name;
> +
> +  stmt = SSA_NAME_DEF_STMT (use);
> +  if (!is_gimple_assign (stmt))
> +    return false;
> +
> +  rhs = gimple_assign_rhs1 (stmt);
> +  if (TREE_CODE (rhs) != ARRAY_REF)
> +    return false;
> +
> +  decl = TREE_OPERAND (rhs, 0);
> +  if (TREE_CODE (decl) != VAR_DECL)
> +    return false;



Also check TREE_STATIC and DECL_ARTIFICIAL flag.


David

> +
> +  name = IDENTIFIER_POINTER (DECL_NAME (decl));
> +  if (strncmp (name, "__gcov", 6))
> +    return false;
> +
> +  return true;
> +}
> +
>  /* Returns true if EXPR contains a ssa name that occurs in an
>     abnormal phi node.  */
>
> @@ -721,7 +783,8 @@ contains_abnormal_ssa_name_p (tree expr)
>    codeclass = TREE_CODE_CLASS (code);
>
>    if (code == SSA_NAME)
> -    return SSA_NAME_OCCURS_IN_ABNORMAL_PHI (expr) != 0;
> +    return SSA_NAME_OCCURS_IN_ABNORMAL_PHI (expr) != 0
> +          || defined_by_gcov_counter (expr);
>
>    if (code == INTEGER_CST
>        || is_gimple_min_invariant (expr))
> Index: testsuite/gcc.dg/profile-generate-4.c
> ===================================================================
> --- testsuite/gcc.dg/profile-generate-4.c       (revision 0)
> +++ testsuite/gcc.dg/profile-generate-4.c       (revision 0)
> @@ -0,0 +1,21 @@
> +/* { dg-do compile { target { i?86-*-* x86_64-*-* } } } */
> +/* { dg-options "-O2 -fprofile-generate -fno-tree-loop-im
> -fdump-tree-ivopts-details-blocks" } */
> +
> +/* Because gcov counter related var has data race for multithread program,
> +   compiler should prevent them from affecting program correctness. So
> +   PROF_edge_counter variable should not be used as induction variable, or
> +   else IVOPT may use such variable to compute loop boundary.  */
> +
> +void *ptr;
> +int N;
> +
> +void foo(void *t) {
> +  int i;
> +  for (i = 0; i < N; i++) {
> +    t = *(void **)t;
> +  }
> +  ptr = t;
> +}
> +
> +/* { dg-final { scan-tree-dump-times "ssa name PROF_edge_counter" 0
> "ivopts"} } */
> +/* { dg-final { cleanup-tree-dump "ivopts" } } */