> On Fri, Aug 29, 2014 at 10:11 AM, Cary Coutant <ccout...@google.com> wrote:
>>> To avoid the unused new discriminator value, I added a map
>>> "found_call_this_line" to track whether a call is the first call in a
>>> source line seen when assigning discriminators. For the first call in
>>> a source line, its discriminator is 0. For the following calls in the
>>> same source line, a new discriminator will be used everytime. The new
>>> patch is attached. Internal perf test and regression test are ok. Is
>>> it ok for google-4_9?
>>
>> This seems overly complex to me. I'd think all you need to do is add a
>> bit to locus_discrim_map (stealing a bit from discriminator ought to
>> be fine) that indicates whether the next call should increment the
>> discriminator or not. Something like this:
>>
>> increase_discriminator_for_locus (location_t locus, bool return_next)
>> {
>> ...
>> if (return_next || (*slot)->needs_increment)
>> {
>> (*slot)->discriminator++;
>> (*slot)->needs_increment = false;
>> }
>> else
>> (*slot)->needs_increment = true;
>> return (*slot)->discriminator;
>> }
>>
>> -cary
Here is the new patch (attached). Regression test passes. Cary, is it ok?
Thanks,
Wei.
ChangeLog:
2014-08-29 Wei Mi <w...@google.com>
* tree-cfg.c (struct locus_discrim_map): New field needs_increment.
(next_discriminator_for_locus): Increase discriminator only when
return_next or needs_increment are true.
(assign_discriminator): Add an actual for next_discriminator_for_locus.
(assign_discriminators): Assign different discriminators for calls
belonging to the same source line.
Index: tree-cfg.c
===================================================================
--- tree-cfg.c (revision 213402)
+++ tree-cfg.c (working copy)
@@ -112,7 +112,14 @@ static struct cfg_stats_d cfg_stats;
struct locus_discrim_map
{
location_t locus;
- int discriminator;
+ /* Different calls belonging to the same source line will be assigned
+ different discriminators. But we want to keep the discriminator of
+ the first call in the same source line to be 0, in order to reduce
+ the .debug_line section size. needs_increment is used for this
+ purpose. It is initialized as false and will be set to true after
+ the first call is seen. */
+ bool needs_increment:1;
+ int discriminator:31;
};
/* Hashtable helpers. */
@@ -914,10 +921,15 @@ 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 next discriminator
+ anyway. If RETURN_NEXT is not true, we may not increase the
+ discriminator if locus_discrim_map::needs_increment is false,
+ which is used when the stmt is the first call stmt in current
+ source line. locus_discrim_map::needs_increment will be set to
+ true after the first call is seen. */
static int
-next_discriminator_for_locus (location_t locus)
+next_discriminator_for_locus (location_t locus, bool return_next)
{
struct locus_discrim_map item;
struct locus_discrim_map **slot;
@@ -932,9 +944,13 @@ next_discriminator_for_locus (location_t
*slot = XNEW (struct locus_discrim_map);
gcc_assert (*slot);
(*slot)->locus = locus;
+ (*slot)->needs_increment = false;
(*slot)->discriminator = 0;
}
- (*slot)->discriminator++;
+ if (return_next || (*slot)->needs_increment)
+ (*slot)->discriminator++;
+ else
+ (*slot)->needs_increment = true;
return (*slot)->discriminator;
}
@@ -974,7 +990,7 @@ assign_discriminator (location_t locus,
if (locus == UNKNOWN_LOCATION)
return;
- discriminator = next_discriminator_for_locus (locus);
+ discriminator = next_discriminator_for_locus (locus, true);
for (gsi = gsi_start_bb (bb); !gsi_end_p (gsi); gsi_next (&gsi))
{
@@ -1009,23 +1025,13 @@ 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)
- {
+ if (gimple_code (stmt) == GIMPLE_CALL)
+ {
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)
- {
+ curr_discr = next_discriminator_for_locus (curr_locus, false);
gimple_set_location (stmt, location_with_discriminator (
- gimple_location (stmt), curr_discr));
+ curr_locus, 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)
