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 <w...@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 <w...@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)
>
>
>
>
>
>
