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 wrote: > Fixed a style error (missing a space before a left parenthesis) > > > On Mon, Jun 30, 2014 at 5:41 PM, Yi Yang wrote: >> Done. >> >> On Mon, Jun 30, 2014 at 5:20 PM, Dehao Chen 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 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 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 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 wrote: >>>> >>> Fixed. (outputting only the integer percentage) >>>> >>> >>>> >>> On Mon, Jun 30, 2014 at 2:20 PM, Yi Yang 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 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 >>>> >>>>> 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 >>>> >>>>>> 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 >>>> >>>>>>> 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 >>>> >>>>>>>> >>>> >>>>>>>> * 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 >>>> >>>>>>>> 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 >>>> >>>>>>>> > 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 >>>> >>>>>>>> >> >>>> >>>>>>>> >> * 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 >>> >>>