From patchwork Fri Oct 4 18:49:49 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Dehao Chen X-Patchwork-Id: 280698 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 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (Client did not present a certificate) by ozlabs.org (Postfix) with ESMTPS id AE3E42C00A7 for ; Sat, 5 Oct 2013 04:49:59 +1000 (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=iieyNjDNWMbQ54rpQC hsgkcrmYpfIa0ky0T5XxN820B01YSeWKIhXkg7XPMPXBoQpR4eSp+Qn12AMmPP8P djli4Q1RuuqIe4EpFJJ2N2SI5otgWlHbx69RYRU101jX697bXd+nXO+DT0WRbZpP 1s1gp2m6BunO8ZE8GPUQbCSZ4= 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=gZv923vVPWk/CvexN3m1YK5V GHY=; b=YpnvDe5REAV9BwtTU5SlGkmtidR+L3FFPJdAlK2n4JnhQY/OmXyiO2th JzIBDWoAgPPbDXwSNlhV7cApK5xln2Ostxjlu3oPeT/BnXvkW2XPR2QEtMtYRW4J FkSyMko+gS8ggbwecHnr31S47TK1UF8La+HYYh+YTHOVD7e1bxo= Received: (qmail 17871 invoked by alias); 4 Oct 2013 18:49:53 -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 17858 invoked by uid 89); 4 Oct 2013 18:49:52 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-3.6 required=5.0 tests=AWL, BAYES_00, RCVD_IN_DNSWL_LOW, RP_MATCHES_RCVD, SPF_PASS autolearn=ham version=3.3.2 X-HELO: mail-vc0-f177.google.com Received: from mail-vc0-f177.google.com (HELO mail-vc0-f177.google.com) (209.85.220.177) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-SHA encrypted) ESMTPS; Fri, 04 Oct 2013 18:49:51 +0000 Received: by mail-vc0-f177.google.com with SMTP id hv10so1881205vcb.22 for ; Fri, 04 Oct 2013 11:49:49 -0700 (PDT) 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=MVdYCjHcnJvxyxQmR4m8xhDy4+U2YDQl1yWnMa3UjG8=; b=Pevy3lm9bYgkgpnSeQQp6iCusu0OagKJSObFHEitNEIlG3bds2yBiVONsfTWL/ZIMq iDms3MNcMkbc4+VEzqXVpTb1gxaEFTWDmX3gKAUV7d0G0oYwrAIj7bGgMcj6hvGFAp9/ /u/Yl2HgeFlylB6zMzl89uagLVzeSoPwcTdvWWyRmDI/Nibw3UwGgc2hWHfTTEIm3ial crvL4FPSm21xSU/sp4iivn5WiU7L2yJaLV3ZFRiTVlnxm7tDIJCuJZWOfzbJTs7atb/Y 6rGo00m9+n3bdpaDZubvMZMZBWY6yYFgUmXlU7ijiEWUWokl6e/Q+Kkmdj9eoBuZfs2r g1MQ== X-Gm-Message-State: ALoCoQl54DDr1ikqI8b+ead1QSqyhXogsby9nRLzLfKxGE6C5dDHmHgwbCEZv3atKwrg1yKa5jilYAUA4GodE825RCSjreYEJj/A4iyRNmGIjhAcvYYqAbL9U0rZYixxOF2YejxLQbteLbRquOcdJy6BW3rCd2fXB3LCksmLLn53Nro29PfL3LchhaRTgJ37C5rpcBRFRQf3bPl8kIjCcxwuMfas6i3hhw== MIME-Version: 1.0 X-Received: by 10.58.235.193 with SMTP id uo1mr13280585vec.6.1380912589409; Fri, 04 Oct 2013 11:49:49 -0700 (PDT) Received: by 10.52.72.136 with HTTP; Fri, 4 Oct 2013 11:49:49 -0700 (PDT) In-Reply-To: References: <20131002160827.GB7181@kam.mff.cuni.cz> <20131002213102.GE7181@kam.mff.cuni.cz> <524EBD0D.9060608@arm.com> Date: Fri, 4 Oct 2013 11:49:49 -0700 Message-ID: Subject: Re: [PATCH] alternative hirate for builtin_expert From: Dehao Chen To: Rong Xu Cc: Ramana Radhakrishnan , Jan Hubicka , GCC Patches , David Li X-IsSubscribed: yes I looked at this problem. Bug updated http://gcc.gnu.org/bugzilla/show_bug.cgi?id=58619 This is a bug when updating block during tree-inline. Basically, it is legal for *n to be NULL. E.g. When gimple_block(id->gimple_call) is NULL, remap_blocks_to_null will be called to set *n to NULL. The problem is that we should check this before calling COMBINE_LOCATION_DATA, which assumes block is not NULL. The following patch can fix the problem: On Fri, Oct 4, 2013 at 11:05 AM, Rong Xu wrote: > My change on the probability of builtin_expect does have an impact on > the partial inline (more outlined functions will get inline back to > the original function). > I think this triggers an existing issue. > Dehao will explain this in his coming email. > > -Rong > > On Fri, Oct 4, 2013 at 6:05 AM, Ramana Radhakrishnan wrote: >> On 10/02/13 23:49, Rong Xu wrote: >>> >>> Here is the new patch. Honaz: Could you take a look? >>> >>> Thanks, >>> >>> -Rong >>> >>> On Wed, Oct 2, 2013 at 2:31 PM, Jan Hubicka wrote: >>>>> >>>>> Thanks for the suggestion. This is much cleaner than to use binary >>>>> parameter. >>>>> >>>>> Just want to make sure I understand it correctly about the orginal >>>>> hitrate: >>>>> you want to retire the hitrate in PRED_BUILTIN_EXPECT and always use >>>>> the one specified in the biniltin-expect-probability parameter. >>>> >>>> >>>> Yes. >>>>> >>>>> >>>>> Should I use 90% as the default? It's hard to fit current value 0.9996 >>>>> in percent form. >>>> >>>> >>>> Yes, 90% seems fine. The original value was set quite arbitrarily and no >>>> real >>>> performance study was made as far as I know except yours. I think users >>>> that >>>> are sure they use expect to gueard completely cold edges may just use >>>> 100% >>>> instead of 0.9996, so I would not worry much about the precision. >>>> >>>> Honza >>>>> >>>>> >>>>> -Rong >>>>>> >>>>>> >>>>>> OK with that change. >>>>>> >>>>>> Honza >> >> >> >> This broke arm-linux-gnueabihf building libstdc++-v3. I haven't dug further >> yet but still reducing the testcase. >> >> See >> >> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=58619 >> >> for details. >> >> Can you please deal with this appropriately ? >> >> regards >> Ramana >> >> Index: gcc/tree-inline.c =================================================================== --- gcc/tree-inline.c (revision 203208) +++ gcc/tree-inline.c (working copy) @@ -2090,7 +2090,10 @@ copy_phis_for_bb (basic_block bb, copy_body_data * n = (tree *) pointer_map_contains (id->decl_map, LOCATION_BLOCK (locus)); gcc_assert (n); - locus = COMBINE_LOCATION_DATA (line_table, locus, *n); + if (*n) + locus = COMBINE_LOCATION_DATA (line_table, locus, *n); + else + locus = LOCATION_LOCUS (locus); } else locus = LOCATION_LOCUS (locus);