No, it is not. This IR is dumped before early inline -- just after
pass_build_cfg. The line number of the deconstructor is marked
according to where its constructor is located, instead of where it is
inserted. This is also problematic.
Wei.
On Thu, Aug 7, 2014 at 12:11 PM, Xinliang David Li <davi...@google.com> wrote:
> 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)
>>
>>
>>
>>
>>
>>
