Fwd: [GOOGLE, AUTOFDO] Assign different discriminators to calls with the same lineno

Wei Mi wmi@google.com
Wed Aug 6 17:23:00 GMT 2014


(Sorry if you received the mail twice because it was not plain text at
first and was rejected by @sourceware.org)

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)



More information about the Gcc-patches mailing list