From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 21819 invoked by alias); 1 Nov 2018 01:11:29 -0000 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 Received: (qmail 21809 invoked by uid 89); 1 Nov 2018 01:11:29 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-2.1 required=5.0 tests=AWL,BAYES_00,FREEMAIL_FROM,RCVD_IN_DNSWL_NONE,SPF_PASS autolearn=ham version=3.3.2 spammy=quality X-HELO: mail-io1-f65.google.com Received: from mail-io1-f65.google.com (HELO mail-io1-f65.google.com) (209.85.166.65) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Thu, 01 Nov 2018 01:11:27 +0000 Received: by mail-io1-f65.google.com with SMTP id k17-v6so11121363ioc.4 for ; Wed, 31 Oct 2018 18:11:27 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=U65mPTG4u2Fk6nN2FKBx61K3sZEnGnzyeEFairDVON0=; b=UKvNpQ/Pb5MSvkbAuOu3CPI7+PklmaF7K9tYKPWNdSYIYbw0HlRWQ0pGiyL9IR/ooM SClJLvRoG4zXb5QbHbKkWNkv6aDIbrFo75i5Vl9Q++Ipqm5tnuy4/R9QgwW1bTS0SQ05 ewKS4SieH1Nx8+2MLbNpshWQP4ap/ivvmvtUKoCu/CF5pGPpfBwXKo+ZTz5AEhcJlwt+ koeNL3si90AfBvvKhZPj/QO9kii26Bia5qUZCxnQPoo8cLLnGkOncfaKVzXJUPqHCZsM syhH5pmvmdPN8ThBi0e8GWBKK9kHRbQe1QyZu3iXDKDIE2H8eBh9MCBY2kCHVxZEihcL XiYQ== MIME-Version: 1.0 References: <7f153787-f390-4661-92aa-06d47cefbbf5.bin.cheng@linux.alibaba.com> In-Reply-To: From: "Bin.Cheng" Date: Thu, 01 Nov 2018 01:11:00 -0000 Message-ID: Subject: Re: [PATCH AutoFDO/2]Treat ZERO as common profile probability/count To: Jeff Law Cc: bin.cheng@linux.alibaba.com, gcc-patches List Content-Type: text/plain; charset="UTF-8" X-IsSubscribed: yes X-SW-Source: 2018-11/txt/msg00001.txt.bz2 On Wed, Oct 31, 2018 at 10:36 PM Jeff Law wrote: > > On 10/31/18 12:30 AM, bin.cheng wrote: > > Hi, > > In new profile probability/count infra, we have different precision quality categories, > > and probabilities/counts of different categories are not supposed to be compared or > > calculated. Though in general is an improvement, it introduces unexpected behavior. > > Specifically, class profile_probablity and profile_count themselves are implemented > > by comparing probabilities/counts against profile_count::zero(). while zero() is of > > profile_precision category, it's always compared different to zero of other precision > > categories including afdo. > > > > I can see two ways fixing this: 1) Treat zero as a common probability/count regardless > > of its category; 2) Provide an "is_zero" method rather than relying on "==" comparison > > against probability_count::zero(). 2) requires lots of code changes so I went with 1) > > in this patch set. This patch doesn't handle "always" but it might be. > > > > This patch also corrects a minor issue where we try to invert an uninitialized value. > > > > Bootstrap and test on x86_64 in patch set. Is it OK? > > > > Thanks, > > bin > > > > 2018-10-31 Bin Cheng > > > > * expmed.c (emit_store_flag_force): Use profile_probability::always. > > * profile-count.h (profile_probability::always): Add parameter. > > (profile_probability::operator==, profile_count::operator==): Treat > > ZERO as common probability/count regardless of its quality. > > (profile_probability::invert): Don't invert uninitialized probability. > > > > I'm really not sure the emit_store_flag_force change is right -- > essentially without external information I can't see how we can pass in > any probability here other than "we don't know" which is > profile_probability::uninitialized IIUC. > > You could potentially make an argument that a 50-50 probability is > reasonable here. That's profile_probability::even. But I just don't > see how profile_probability::always is right here. Thanks, I realized I mis-spotted that piece of code. Will discard the part. Thanks, bin > > jeff