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 <ahyan...@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 <ahyan...@google.com> wrote: >> Fixed. (outputting only the integer percentage) >> >> On Mon, Jun 30, 2014 at 2:20 PM, Yi Yang <ahyan...@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 <de...@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 <ahyan...@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 <de...@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 <ahyan...@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 <ahyan...@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 <davi...@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 <ahyan...@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 <ahyan...@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
