This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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


Per offline discussion,
* do not export function start line number. Instead, hash branch
offset and discriminator into the "function_hash" (renamed to just
"hash" to clarify)
* protect all operations about the new EDGE_PREDICTED_BY_EXPECT flag
with flag_check_branch_annotation.

On Mon, Jun 30, 2014 at 5:42 PM, Yi Yang <ahyangyi@google.com> wrote:
> Fixed a style error (missing a space before a left parenthesis)
>
>
> On Mon, Jun 30, 2014 at 5:41 PM, Yi Yang <ahyangyi@google.com> wrote:
>> Done.
>>
>> On Mon, Jun 30, 2014 at 5:20 PM, Dehao Chen <dehao@google.com> wrote:
>>> For get_locus_information, can you cal get_inline_stack and directly use its
>>> output to get the function name instead?
>>>
>>> Dehao
>>>
>>>
>>> On Mon, Jun 30, 2014 at 4:47 PM, Yi Yang <ahyangyi@google.com> wrote:
>>>>
>>>> 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


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]