Patchwork [GOOGLE] Assign discriminators for different callsites at a same line within one BB

login
register
mail settings
Submitter Dehao Chen
Date Aug. 20, 2013, 11:19 p.m.
Message ID <CAO2gOZWKzGzmiR-DFS69mVDvziWYAoUY0-SwM9kaOToF0w7WvQ@mail.gmail.com>
Download mbox | patch
Permalink /patch/268658/
State New
Headers show

Comments

Dehao Chen - Aug. 20, 2013, 11:19 p.m.
You are right, we need discriminator for non-CALL stmts too. Patch updated:

On Tue, Aug 20, 2013 at 2:54 PM, Cary Coutant <ccoutant@google.com> wrote:
>> This patch assigns discriminators for different callsites within the
>> same BB. This is needed for accurate profile attribution in AutoFDO.
>>
>> Testing on going.
>>
>> OK for google branches if test pass?
>
>> +      for (gsi = gsi_start_bb (bb); !gsi_end_p (gsi); gsi_next (&gsi))
>> + {
>> +  gimple stmt = gsi_stmt (gsi);
>> +  if (gimple_code (stmt) != GIMPLE_CALL)
>> +    continue;
>> +  if (curr_locus == UNKNOWN_LOCATION ||
>> +      !same_line_p (curr_locus, gimple_location (stmt)))
>> +    curr_locus = gimple_location (stmt);
>> +  else
>> +    gimple_set_location (stmt, location_with_discriminator (
>> + gimple_location (stmt),
>> + next_discriminator_for_locus (gimple_location (stmt))));
>> + }
>
> Please add a comment above this block explaining what it's doing. Are
> you sure it's sufficient to just assign a new discriminator to the
> CALL stmt, and not to the subsequent stmts after the CALL?
>
> -cary
Cary Coutant - Aug. 21, 2013, 4:49 p.m.
> You are right, we need discriminator for non-CALL stmts too. Patch updated:

OK for google branches. Thanks!

-cary

Patch

Index: gcc/tree-cfg.c
===================================================================
--- gcc/tree-cfg.c (revision 201858)
+++ gcc/tree-cfg.c (working copy)
@@ -781,9 +781,37 @@  assign_discriminators (void)
     {
       edge e;
       edge_iterator ei;
+      gimple_stmt_iterator gsi;
       gimple last = last_stmt (bb);
       location_t locus = last ? gimple_location (last) : UNKNOWN_LOCATION;
+      location_t curr_locus = UNKNOWN_LOCATION;
+      int curr_discr = 0;

+      /* Traverse the basic block, if two function calls within a basic block
+ are mapped to a same line, assign a new discriminator because a call
+ stmt could be a split point of a basic block.  */
+      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)))
+    {
+      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));
+    }
+  /* 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)
  continue;