and for google/gcc4_9 too. David
On Tue, Jul 1, 2014 at 1:16 PM, Dehao Chen <de...@google.com> wrote: > OK for google-4_8 after testing. > > Thanks, > Dehao > > On Tue, Jul 1, 2014 at 1:04 PM, Yi Yang <ahyan...@google.com> wrote: >> 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 <ahyan...@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 <ahyan...@google.com> wrote: >>>> Done. >>>> >>>> On Mon, Jun 30, 2014 at 5:20 PM, Dehao Chen <de...@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 <ahyan...@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 <de...@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 <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 >>>>> >>>>>
