From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 42166 invoked by alias); 31 Oct 2018 09:35:37 -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 42036 invoked by uid 89); 31 Oct 2018 09:35:36 -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-it1-f195.google.com Received: from mail-it1-f195.google.com (HELO mail-it1-f195.google.com) (209.85.166.195) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 31 Oct 2018 09:35:34 +0000 Received: by mail-it1-f195.google.com with SMTP id e74-v6so17364137ita.2 for ; Wed, 31 Oct 2018 02:35:34 -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=a/Bqc0xG2lBW6UH83GFGqBQCIXmxNxE2thnu+G51q2k=; b=BbuoH2u9jYShr/j/mJh91cj26IZRGsLWB1wL4Ux0yElchznDlvI6W1IFMlf5XZffzP 2hNk1CObxD/FIuAUrU0Kz/u/FJ0F+XED8JX+Qccvcok8L+Oja/6H1qwsHEOZSL4OAV71 HgRiguYc4bYy8y431MFrU4Trx8ngMzVKeZbnlvZ0Zd++lSI2RlTh5FiHFTSKAcXllIf2 MINC3pAZz1O7KhJWMruvZ8tW2h/OrxZEcIajY6dXfZrpZj0xqI9G2vv0SYgvdw8wx7VG 3WKnLvC7KRvnw/y5a+R2InEj2i1eUi5dMqkBkMJJsRiOiHpJa3NwnXjNqiW4ab3Zcy1y Zckw== MIME-Version: 1.0 References: <7f153787-f390-4661-92aa-06d47cefbbf5.bin.cheng@linux.alibaba.com> In-Reply-To: From: "Bin.Cheng" Date: Wed, 31 Oct 2018 09:57:00 -0000 Message-ID: Subject: Re: [PATCH AutoFDO/2]Treat ZERO as common profile probability/count To: Richard Guenther Cc: bin.cheng@linux.alibaba.com, Jan Hubicka , gcc-patches List Content-Type: text/plain; charset="UTF-8" X-IsSubscribed: yes X-SW-Source: 2018-10/txt/msg02005.txt.bz2 On Wed, Oct 31, 2018 at 5:11 PM Richard Biener wrote: > > On Wed, Oct 31, 2018 at 7: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? > > I'll defer on the emit_store_flag_force change, likewise for the zero > handling in > compares - I don't think zeros of different qualities should compare equal. > Would compares against ::always() not have the very same issue? > Likewise ::even(), > ::likely(), etc.? Those always get guessed quality. Yes, these values also affected if compared with precise category, but zero is the major issue. So 2) makes more sense when checking if a profile count is_zero/is_likely/is_always etc. regardless of its categories. Once with Honza's input, I can do some experiments. Thanks, bin > > The invert change looks OK to me. The related change to the always() API would > suggest to replace guessed_always() with always (guessed) and also do similar > changes throughout the whole API... > > Honza? > > Thanks, > Richard. > > > > 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.