From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 19016 invoked by alias); 29 Nov 2012 16:46:00 -0000 Received: (qmail 18978 invoked by uid 22791); 29 Nov 2012 16:45:54 -0000 X-SWARE-Spam-Status: No, hits=-5.8 required=5.0 tests=AWL,BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,KHOP_RCVD_TRUST,KHOP_THREADED,RCVD_IN_DNSWL_LOW,RCVD_IN_HOSTKARMA_YE,RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from mail-da0-f47.google.com (HELO mail-da0-f47.google.com) (209.85.210.47) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Thu, 29 Nov 2012 16:45:35 +0000 Received: by mail-da0-f47.google.com with SMTP id s35so3151112dak.20 for ; Thu, 29 Nov 2012 08:45:34 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20120113; h=mime-version:in-reply-to:references:date:message-id:subject:from:to :cc:content-type:x-gm-message-state; bh=eI9/0lE9z2kyz44iZG1a6f0nmJLONuecfdF/2l8UqbE=; b=pIkptQxYSf8DM6Z0NKJSC7mOkEDlWIF/0WW7C/cJf7MYDJqxJC9Z5sHHl1mj1II1gD r0zQdDTqIBlCxk3H5BKgmRDdnVc2Q8d/SOZQdbDUzfv1OyjKqL+groXi8XGdzgW5mwoL nXRp3hTAkSUWxK+J/vwqdB60+TupyMkhbyTd1Nvti4nxx5MeLvcZptDZNDuzbAb9Icyx NvjZqor2Hcvkagx1RYBaV+zPXYXlrc3lUTKZz0DGj2vhvsX0vDS+hKjOCY2iR0Gj7Epr m+5gYqhf7aKfSlo5C8qDzVVEIIsNXnNEDic50e/NXfTMH4LG0b4tiU5Q7IT7fUIX0VHJ EK6w== MIME-Version: 1.0 Received: by 10.68.136.135 with SMTP id qa7mr69396778pbb.157.1354207534123; Thu, 29 Nov 2012 08:45:34 -0800 (PST) Received: by 10.68.6.106 with HTTP; Thu, 29 Nov 2012 08:45:34 -0800 (PST) In-Reply-To: <20121129161744.GA26688@atrey.karlin.mff.cuni.cz> References: <20121129041116.DB81E61422@tjsboxrox.mtv.corp.google.com> <20121129161744.GA26688@atrey.karlin.mff.cuni.cz> Date: Thu, 29 Nov 2012 16:46:00 -0000 Message-ID: Subject: Re: [PATCH] Stream profile summary histogram through LTO files (issue6782131) From: Teresa Johnson To: Jan Hubicka Cc: reply@codereview.appspotmail.com, "gcc-patches@gcc.gnu.org" Content-Type: text/plain; charset=ISO-8859-1 X-Gm-Message-State: ALoCoQmTJMDSP4h377BvaQAKjqK+MwOcdlNsduMRsplYzuiqboOUXG/BOGrLWrCje7zKO6Diq0WnIThOJdcWLRkcRS/HtQ9ozENDDnD+iP0/yvZqDEXXX0bHXi//tU6M66JIkiqVnek21lQpP/NXXJOdjpV/vwt+GmU8Irlq/ssJI6Y73/GMpXVrXiuP5PABq3afLKHCWGrS X-IsSubscribed: yes Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org X-SW-Source: 2012-11/txt/msg02459.txt.bz2 On Thu, Nov 29, 2012 at 8:17 AM, Jan Hubicka wrote: >> This patch ensures that the histograms from the profile summary are streamed >> through the LTO files so that the working set can be computed for use in >> downstream optimizations. >> >> Bootstrapped and tested on x86_64-unknown-linux-gnu. Ok for trunk? >> >> Thanks, >> Teresa >> >> 2012-11-28 Teresa Johnson >> >> * lto-cgraph.c (output_profile_summary): Stream out sum_all >> and histogram. >> (input_profile_summary): Stream in sum_all and histogram. >> (merge_profile_summaries): Merge sum_all and histogram. >> (input_symtab): Call compute_working_sets after merging >> summaries. >> * gcov-io.c (gcov_histo_index): Make extern for compiler. >> * gcov-io.h (gcov_histo_index): Ditto. >> * profile.c (compute_working_sets): Remove static keyword. >> * profile.h (compute_working_sets): Ditto. > > OK. >> >> Index: lto-cgraph.c >> =================================================================== >> --- lto-cgraph.c (revision 193909) >> +++ lto-cgraph.c (working copy) >> @@ -46,6 +46,7 @@ along with GCC; see the file COPYING3. If not see >> #include "tree-streamer.h" >> #include "gcov-io.h" >> #include "tree-pass.h" >> +#include "profile.h" > > Please update dependencies in Makefile.in ok. >> + /* Count number of non-zero histogram entries, and fill in a bit vector >> + of non-zero indices. */ >> + counters. */ >> + for (bv_ix = 0; bv_ix < GCOV_HISTOGRAM_BITVECTOR_SIZE; bv_ix++) >> + histo_bitvector[bv_ix] = 0; >> + for (h_ix = 0; h_ix < GCOV_HISTOGRAM_SIZE; h_ix++) >> + { >> + if (profile_info->histogram[h_ix].num_counters > 0) >> + { >> + histo_bitvector[h_ix / 32] |= 1 << (h_ix % 32); >> + h_cnt++; >> + } > > I think this would be more readable if you just produced a bitpack instead of doing it > by hand, like into gcov-io. I assume you mean use the bitpack streaming functionality used other places in lto-cgraph.c, and not change the way it is done in gcov-io.c where this was cloned from? I will change the lto-cgraph.c code to use the bitpacking. >> + lto_gcov_summary.sum_all = MAX (lto_gcov_summary.sum_all, >> + (file_data->profile_info.sum_all >> + * scale >> + + REG_BR_PROB_BASE / 2) >> + / REG_BR_PROB_BASE); > > Use RDIV for the scaling. ok. This was cloned from the way we do other scalings in the same function, I will change them all to use RDIV. >> -#if IN_LIBGCOV || !IN_GCOV >> +#if !IN_GCOV >> /* Determine the index into histogram for VALUE. */ >> >> -static unsigned >> +GCOV_LINKAGE unsigned > I would probably go around the trouble of declaring this static in GCOV, > so it is inlined at we do not bload libgcov more than needed. Do you mean leave it static when IN_LIBGCOV? It isn't included when IN_GCOV. I just need it extern when in the compiler. So do you mean make it static when IN_LIBGCOV and GCOV_LINKAGE (i.e. extern) when !IN_LIBGCOV? Thanks, Teresa > > Thanks, > Honza -- Teresa Johnson | Software Engineer | tejohnson@google.com | 408-460-2413