Re: [GOOGLE] Report the difference between profiled and guessed or annotated branch probabilities.

[Yi Yang <ahyangyi at google dot com>]

Removed fill_invalid_locus_information. Change the function call to a
return statement.

On Mon, Jun 30, 2014 at 4:42 PM, Dehao Chen <dehao@google.com> wrote:
> There is no need for fill_invalid_locus_information, just initialize
> every field to 0, and if it's unknown location, no need to output this
> line.
>
> Dehao
>
> On Mon, Jun 30, 2014 at 4:26 PM, Yi Yang <ahyangyi@google.com> wrote:
>> Instead of storing percentages of the branch probabilities, store them
>> times REG_BR_PROB_BASE.
>>
>> On Mon, Jun 30, 2014 at 3:26 PM, Yi Yang <ahyangyi@google.com> wrote:
>>> Fixed. (outputting only the integer percentage)
>>>
>>> On Mon, Jun 30, 2014 at 2:20 PM, Yi Yang <ahyangyi@google.com> wrote:
>>>> This is intermediate result, which is meant to be consumed by further
>>>> post-processing. For this reason I'd prefer to put a number without
>>>> that percentage sign.
>>>>
>>>> I'd just output (int)(probability*100000000+0.5). Does this look good
>>>> for you? Or maybe change that to 1000000 since six digits are more
>>>> than enough. I don't see a reason to intentionally drop precision
>>>> though.
>>>>
>>>> Note that for the actual probability, the best way to store it is to
>>>> store the edge count, since the probability is just
>>>> edge_count/bb_count. But this causes disparity in the formats of the
>>>> two probabilities.
>>>>
>>>> On Mon, Jun 30, 2014 at 2:12 PM, Dehao Chen <dehao@google.com> wrote:
>>>>> Let's use %d to replace %f (manual conversion, let's do xx%).
>>>>>
>>>>> Dehao
>>>>>
>>>>> On Mon, Jun 30, 2014 at 2:06 PM, Yi Yang <ahyangyi@google.com> wrote:
>>>>>> Fixed.
>>>>>>
>>>>>> Also, I spotted some warnings caused by me using "%lf"s in snprintf().
>>>>>> I changed these to "%f" and tested.
>>>>>>
>>>>>>
>>>>>> On Mon, Jun 30, 2014 at 1:49 PM, Dehao Chen <dehao@google.com> wrote:
>>>>>>> You don't need extra space to store file name in locus_information_t.
>>>>>>> Use pointer instead.
>>>>>>>
>>>>>>> Dehao
>>>>>>>
>>>>>>>
>>>>>>> On Mon, Jun 30, 2014 at 1:36 PM, Yi Yang <ahyangyi@google.com> wrote:
>>>>>>>>
>>>>>>>> I refactored the code and added comments. A bug (prematurely breaking
>>>>>>>> from a loop) was fixed during the refactoring.
>>>>>>>>
>>>>>>>> (My last mail was wrongly set to HTML instead of plain text. I
>>>>>>>> apologize for that.)
>>>>>>>>
>>>>>>>> 2014-06-30  Yi Yang  <ahyangyi@google.com>
>>>>>>>>
>>>>>>>>     * auto-profile.c (get_locus_information)
>>>>>>>>     (fill_invalid_locus_information, record_branch_prediction_results)
>>>>>>>>     (afdo_calculate_branch_prob, afdo_annotate_cfg): Main comparison and
>>>>>>>>     reporting logic.
>>>>>>>>     * cfg-flags.def (PREDICTED_BY_EXPECT): Add an extra flag representing
>>>>>>>>     an edge's probability is predicted by annotations.
>>>>>>>>     * predict.c (combine_predictions_for_bb): Set up the extra flag on an
>>>>>>>>     edge when appropriate.
>>>>>>>>     * common.opt (fcheck-branch-annotation)
>>>>>>>>     (fcheck-branch-annotation-threshold=): Add an extra GCC option to turn
>>>>>>>>     on report
>>>>>>>>
>>>>>>>> On Fri, Jun 27, 2014 at 3:20 PM, Xinliang David Li <davidxl@google.com> wrote:
>>>>>>>> > Hi Yi,
>>>>>>>> >
>>>>>>>> > 1) please add comments before new functions as documentation -- follow
>>>>>>>> > the coding style guideline
>>>>>>>> > 2) missing documenation on the new flags (pointed out by Gerald)
>>>>>>>> > 3) Please refactor the check code in afdo_calculate_branch_prob into a
>>>>>>>> > helper function
>>>>>>>> >
>>>>>>>> > 4) the change log is not needed for google branches, but if provided,
>>>>>>>> > the format should follow the style guide (e.g, function name in () ).
>>>>>>>> >
>>>>>>>> > David
>>>>>>>> >
>>>>>>>> >
>>>>>>>> > On Fri, Jun 27, 2014 at 11:07 AM, Yi Yang <ahyangyi@google.com> wrote:
>>>>>>>> >> Hi,
>>>>>>>> >>
>>>>>>>> >> This patch adds an option. When the option is enabled, GCC will add a
>>>>>>>> >> record about it in an elf section called
>>>>>>>> >> ".gnu.switches.text.branch.annotation" for every branch.
>>>>>>>> >>
>>>>>>>> >> gcc/
>>>>>>>> >>
>>>>>>>> >> 2014-06-27 Yi Yang <ahyangyi@google.com>
>>>>>>>> >>
>>>>>>>> >>         * auto-profile.c: Main comparison and reporting logic.
>>>>>>>> >>         * cfg-flags.def: Add an extra flag representing an edge's
>>>>>>>> >> probability is predicted by annotations.
>>>>>>>> >>         * predict.c: Set up the extra flag on an edge when appropriate.
>>>>>>>> >>         * common.opt: Add an extra GCC option to turn on this report mechanism

Attachment: 0001-Report-the-difference-between-profiled-and-guessed-o.patch.txt
Description: Text document