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: [GOOGLE, AUTOFDO] Assign different discriminators to calls with the same lineno


Is this

[1.cc : 179:64] Reader::~Reader (&version);

from an inline instance?

David

On Wed, Aug 6, 2014 at 10:18 AM, Wei Mi <wmi@google.com> wrote:
> We saw bb like this in the IR dump after pass_build_cfg:
>
>   <bb 21>:
>   [1.cc : 205:45] D.332088 = table->_vptr.Table;
>   [1.cc : 205:45] D.332134 = D.332088 + 104;
>   [1.cc : 205:45] D.332135 = [1.cc : 205] *D.332134;
>   [1.cc : 205:45] D.332092 = [1.cc : 205] &this->cp_stream_;
>   [1.cc : 205:46] OBJ_TYPE_REF(D.332135;(struct Table)table->13) (table,
> cp_arg, D.332092);  // indirect call
>   [1.cc : 179:64] Reader::~Reader (&version);
>   [1.cc : 205:46] Switcher::~Switcher (&tcswr);
>
> The indirect call above has the same source lineno with "Switcher::~Switcher
> (&tcswr);", but they have no discriminator so they cannot be discriminated
> in autofdo. This causes the problem that autofdo mistakenly regards
> "Switcher::~Switcher (&tcswr);" as a target of the indirect call above, and
> makes a wrong promotion.
>
> The existing code has the logic to assign different discriminators to calls
> with the same lineno, but it only works when the lineno in a bb is
> monotonical. In this case, there is another stmt with lineno 179 between the
> indirect call and "Switcher::~Switcher (&tcswr);" (both with lineno 205), so
> existing code will not assign different discriminators for them.
>
> The patch is to assign discriminators for calls with the same lineno anyway.
>
> regression test is going. internal perf test for autofdo shows a little
> improvement. Ok for google-4_9 if regression pass?
>
> Thanks,
> Wei.
>
> ChangeLog:
>
> 2014-08-06  Wei Mi  <wmi@google.com>
>
>         * tree-cfg.c (increase_discriminator_for_locus): It was
>         next_discriminator_for_locus. Add a param "return_next".
>         (next_discriminator_for_locus): Renamed.
>         (assign_discriminator): Use the renamed func.
>         (assign_discriminators): Assign different discriminators
>         for calls with the same lineno.
>
>
> Index: tree-cfg.c
> ===================================================================
> --- tree-cfg.c  (revision 213402)
> +++ tree-cfg.c  (working copy)
> @@ -914,10 +914,12 @@ make_edges (void)
>  /* Find the next available discriminator value for LOCUS.  The
>     discriminator distinguishes among several basic blocks that
>     share a common locus, allowing for more accurate sample-based
> -   profiling.  */
> +   profiling. If RETURN_NEXT is true, return the discriminator
> +   value after the increase, else return the discriminator value
> +   before the increase.  */
>
>  static int
> -next_discriminator_for_locus (location_t locus)
> +increase_discriminator_for_locus (location_t locus, bool return_next)
>  {
>    struct locus_discrim_map item;
>    struct locus_discrim_map **slot;
> @@ -934,8 +936,10 @@ next_discriminator_for_locus (location_t
>        (*slot)->locus = locus;
>        (*slot)->discriminator = 0;
>      }
> +
>    (*slot)->discriminator++;
> -  return (*slot)->discriminator;
> +  return return_next ? (*slot)->discriminator
> +                    : (*slot)->discriminator - 1;
>  }
>
>  /* Return TRUE if LOCUS1 and LOCUS2 refer to the same source line.  */
> @@ -974,7 +978,7 @@ assign_discriminator (location_t locus,
>    if (locus == UNKNOWN_LOCATION)
>      return;
>
> -  discriminator = next_discriminator_for_locus (locus);
> +  discriminator = increase_discriminator_for_locus (locus, true);
>
>    for (gsi = gsi_start_bb (bb); !gsi_end_p (gsi); gsi_next (&gsi))
>      {
> @@ -1009,23 +1013,16 @@ assign_discriminators (void)
>        for (gsi = gsi_start_bb (bb); !gsi_end_p (gsi); gsi_next (&gsi))
>         {
>           gimple stmt = gsi_stmt (gsi);
> -         if (curr_locus == UNKNOWN_LOCATION)
> -           {
> -             curr_locus = gimple_location (stmt);
> -           }
> -         else if (!same_line_p (curr_locus, gimple_location (stmt)))
> +         if (gimple_code (stmt) == GIMPLE_CALL)
>             {
>               curr_locus = gimple_location (stmt);
> -             curr_discr = 0;
> -           }
> -         else if (curr_discr != 0)
> -           {
> -             gimple_set_location (stmt, location_with_discriminator (
> -                 gimple_location (stmt), curr_discr));
> +             /* return the current discriminator first, then increase the
> +                discriminator for next call.  */
> +             curr_discr = increase_discriminator_for_locus (curr_locus,
> false);
> +             if (curr_discr != 0)
> +               gimple_set_location (stmt, location_with_discriminator (
> +                   gimple_location (stmt), curr_discr));
>             }
> -         /* Allocate a new discriminator for CALL stmt.  */
> -         if (gimple_code (stmt) == GIMPLE_CALL)
> -           curr_discr = next_discriminator_for_locus (curr_locus);
>         }
>
>        if (locus == UNKNOWN_LOCATION)
>
>
>
>
>
>


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