From patchwork Wed Nov 13 14:44:45 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Teresa Johnson X-Patchwork-Id: 290955 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by ozlabs.org (Postfix) with ESMTPS id 6787D2C0087 for ; Thu, 14 Nov 2013 01:45:06 +1100 (EST) DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender :mime-version:in-reply-to:references:date:message-id:subject :from:to:cc:content-type; q=dns; s=default; b=MGmUs6vE/i28xAsle0 IaIXAI5Jem2uH85AhcKXsXGToEnT5FPK8DP6qlLK9K816w8llvMdYxC3Z/NLVZ1Z Bmi7ou8bqpO2qUnIegoExi+2todMKv0eVM8YDdK01SFg8ha+MGLYK1RAHAwPGfWh 8eWfEkGd/kO0kuH+ku/eQgpT0= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender :mime-version:in-reply-to:references:date:message-id:subject :from:to:cc:content-type; s=default; bh=Eb71gexx0oFM4sIFqo50wn3J 7D0=; b=uuyZwFeR2yQMZNoNQ7IPdSq16uhfDLi2/Fv+QweuAZ/bER60YAUxTv9F UBMXTWkm+rBkVz0JtWoQqMRN75+Wq2HtQ+x9x5Z3FsTzk3JHhZXPUTOCcKsHBxpX E8ZHDfTnKd8wBTexkeD7fYL+5RcL/AyIvD5Wi7aT01ZF0xnBiwg= Received: (qmail 29920 invoked by alias); 13 Nov 2013 14:44:55 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org Received: (qmail 29908 invoked by uid 89); 13 Nov 2013 14:44:55 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.4 required=5.0 tests=AWL, BAYES_50, RCVD_IN_DNSWL_LOW, RDNS_NONE, SPF_PASS, URIBL_BLOCKED autolearn=no version=3.3.2 X-HELO: mail-qc0-f179.google.com Received: from Unknown (HELO mail-qc0-f179.google.com) (209.85.216.179) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-SHA encrypted) ESMTPS; Wed, 13 Nov 2013 14:44:53 +0000 Received: by mail-qc0-f179.google.com with SMTP id k18so269555qcv.24 for ; Wed, 13 Nov 2013 06:44:45 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:in-reply-to:references:date :message-id:subject:from:to:cc:content-type; bh=vSwNQ6lA9K/U6tNy5mprknNxshnrF2htpTrgF+uS0A8=; b=SRKAPKiOtof+UHtzVQvw5kkyoUKoCceiLNVFEuHtW+clSarsuxzTMM6VA8V0Mo4tRW 0t0P/6qEQLjvoNDy2I2onx3TbYobdNgZmx0XBFxNz82HTqmUPbmKNN/wED7C/B7i92Zt vTkv+cHBVYQRWaT5QWz0rujgxG6JwdneLu8GIwOOo3CA/l8oIx539hgZLbRNQQD4bFnR smrJTq/VYbLS6EDhSx0I+A+xkEHqHGORqDrbJc61XZ21G56VWQdPn2SEV+6xoSjMn1Mi 7bz8dIe9EGs5OsYX4TbuvT0aw/CnauFG78ke68GjDODdO/+WPw/e9zaWSq+4kNMIlYY2 nDBQ== X-Gm-Message-State: ALoCoQm4e0TM1eehUu6h0rmrUc6OAJDmFuhHw6dLWunA0R4iGEaC+gEOVyC9HAabhOo8H9mP9EBomjWNHBNuBcIb77PROflBc9VfB9TmfgwfYKJ0wg4aH2yVAmT8iGyZxSzBC1VsDn0Nr0mvYyJQzyqo6ae/gnMo8B/pq7qbROqAMokrM4V9LVHC/D6j2hCAktQhNWEUa901az66kO5Y4xZsJqc59mUzbQ== MIME-Version: 1.0 X-Received: by 10.49.51.103 with SMTP id j7mr65828204qeo.29.1384353885477; Wed, 13 Nov 2013 06:44:45 -0800 (PST) Received: by 10.229.117.69 with HTTP; Wed, 13 Nov 2013 06:44:45 -0800 (PST) In-Reply-To: <20131112223549.GA22297@kam.mff.cuni.cz> References: <20131111152309.GC4119@atrey.karlin.mff.cuni.cz> <20131111162226.GD4119@atrey.karlin.mff.cuni.cz> <20131111171307.GA22469@atrey.karlin.mff.cuni.cz> <20131112213306.GA14050@kam.mff.cuni.cz> <20131112223549.GA22297@kam.mff.cuni.cz> Date: Wed, 13 Nov 2013 06:44:45 -0800 Message-ID: Subject: Re: [PATCH] decide edge's hotness when there is profile info From: Teresa Johnson To: Jan Hubicka Cc: Dehao Chen , Xinliang David Li , GCC Patches , Uros Bizjak X-IsSubscribed: yes On Tue, Nov 12, 2013 at 2:35 PM, Jan Hubicka wrote: >> On Tue, Nov 12, 2013 at 1:33 PM, Jan Hubicka wrote: >> >> More info on the lto bootstrap failure: >> >> >> >> /usr/local/google/home/tejohnson/gcc_trunk_9/libiberty/pex-unix.c:790:1: >> >> warning: Missing counts for called function pex_child_error.isra.1/73 >> >> [enabled by default] >> >> } >> >> >> >> This is an error handling routine that invokes _exit. Looking at the >> >> ipa-profile dump, I can see that all the counts read in for >> >> pex_child_error have the value 0. But there is a non-zero callsite, >> >> that not surprisingly complains of a profile insanity in the bb's >> >> outgoing edge weights, since pex_child_error never returns: >> >> >> >> ;; basic block 38, loop depth 1, count 192, freq 5000 >> >> ;; Invalid sum of outgoing counts 0, should be 192 >> >> ... >> >> pex_child_error.isra.1D.5005 (_92, executable_40(D), >> >> [/usr/local/google/home/tejohnson/gcc_trunk_9/libiberty/pex-unix.c : >> >> 677] "execv", _91); >> >> ;; succ: 3 [100.0%] (ABNORMAL,DFS_BACK,IRREDUCIBLE_LOOP,EXECUTABLE) >> >> >> >> Not sure why the counts were all 0 for this routine though, I wouldn't >> >> think the early exit should prevent us from updating the counts. But >> >> IMO the best thing to do here is to issue a warning, since I don't >> >> want more issues like this to cause compile errors when we handled >> >> them fine before. >> >> >> >> Let me know if the patch below is ok. >> > >> > OK, so _exit actually makes us to miss streaming of the path leading to the >> > error message? Handling of vfork is somewhat broken, as I noticed with Martin >> > Liska. One problem is that gcov_exit is not clearing the counts. With vfork >> > being used in addition to execvs we stream out the data but the counts stays in >> > the oriignal process memory and then are streamed again. If execve fails and >> > leads to _exit I think we can get the miscompare you see. >> > >> > Does calling gcov_clear within gcov_exit help? (It is what I have in our tree) >> >> I thought this was already handled by the __gcov_execv wrapper around >> execv which invokes __gcov_flush that in turns does a gcov_exit >> followed by gcov_clear? >> >> I think the issue is that we register gcov_exit with atexit(), but >> _exit does not execute any atexit functions by definition. So that >> would explain why the counters from pex_child_error didn't get dumped. >> The caller bb invokes execv before invoking pex_child_error: >> execv (executable, to_ptr32 (argv)); >> pex_child_error (obj, executable, "execv", errno); >> So the counters of the caller bb are dumped (hence the caller bb has >> non-zero counts) and cleared, and the pex_child_error does not dump >> its counters since it uses _exit instead of exit and therefore doesn't >> invoke gcov_exit atexit. > > Hmm, I see, the problem here is that execv gets BB splitted (and profile is correct) > but before we get into ipa_profile the BBs get re-merged and we incorrectly put > count of 1 to pex_child_error. > > I suppose this will always happen when you have function terminating program > (execve) before other function call. Perhaps we can warn only when the difference in counts > is greater than number of train runs? Ok, that sounds good. Here is the new patch. Is this ok for trunk if testing (bootstrap regression and lto profiledbootstrap) succeeds? Thanks, Teresa 2013-11-13 Teresa Johnson * predict.c (drop_profile): Error is currently too strict. (handle_missing_profiles): Pass call_count to drop_profile. > > Honza Index: predict.c =================================================================== --- predict.c (revision 204704) +++ predict.c (working copy) @@ -2766,12 +2766,17 @@ estimate_loops (void) } /* Drop the profile for NODE to guessed, and update its frequency based on - whether it is expected to be HOT. */ + whether it is expected to be hot given the CALL_COUNT. */ static void -drop_profile (struct cgraph_node *node, bool hot) +drop_profile (struct cgraph_node *node, gcov_type call_count) { struct function *fn = DECL_STRUCT_FUNCTION (node->decl); + /* In the case where this was called by another function with a + dropped profile, call_count will be 0. Since there are no + non-zero call counts to this function, we don't know for sure + whether it is hot, and therefore it will be marked normal below. */ + bool hot = maybe_hot_count_p (NULL, call_count); if (dump_file) fprintf (dump_file, @@ -2781,8 +2786,13 @@ static void /* We only expect to miss profiles for functions that are reached via non-zero call edges in cases where the function may have been linked from another module or library (COMDATs and extern - templates). See the comments below for handle_missing_profiles. */ - if (!DECL_COMDAT (node->decl) && !DECL_EXTERNAL (node->decl)) + templates). See the comments below for handle_missing_profiles. + Also, only warn in cases where the missing counts exceed the + number of training runs. In certain cases with an execv followed + by a no-return call the profile for the no-return call is not + dumped and there can be a mismatch. */ + if (!DECL_COMDAT (node->decl) && !DECL_EXTERNAL (node->decl) + && call_count > profile_info->runs) { if (flag_profile_correction) { @@ -2792,8 +2802,8 @@ static void cgraph_node_name (node), node->order); } else - error ("Missing counts for called function %s/%i", - cgraph_node_name (node), node->order); + warning (0, "Missing counts for called function %s/%i", + cgraph_node_name (node), node->order); } profile_status_for_function (fn) @@ -2839,9 +2849,7 @@ handle_missing_profiles (void) && fn && fn->cfg && (call_count * unlikely_count_fraction >= profile_info->runs)) { - bool maybe_hot = maybe_hot_count_p (NULL, call_count); - - drop_profile (node, maybe_hot); + drop_profile (node, call_count); worklist.safe_push (node); } } @@ -2863,11 +2871,7 @@ handle_missing_profiles (void) if (DECL_COMDAT (callee->decl) && fn && fn->cfg && profile_status_for_function (fn) == PROFILE_READ) { - /* Since there are no non-0 call counts to this function, - we don't know for sure whether it is hot. Indicate to - the drop_profile routine that function should be marked - normal, rather than hot. */ - drop_profile (node, false); + drop_profile (node, 0); worklist.safe_push (callee); } }