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 >>>> >>>>
