From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 75993 invoked by alias); 6 May 2019 10:11:19 -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 75984 invoked by uid 89); 6 May 2019 10:11:19 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-4.4 required=5.0 tests=AWL,BAYES_00,FREEMAIL_FROM,GIT_PATCH_2,RCVD_IN_DNSWL_NONE,SPF_PASS autolearn=ham version=3.3.1 spammy=sk:infinit, costs, Cheng, cheng X-HELO: mail-lf1-f67.google.com Received: from mail-lf1-f67.google.com (HELO mail-lf1-f67.google.com) (209.85.167.67) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 06 May 2019 10:11:18 +0000 Received: by mail-lf1-f67.google.com with SMTP id n134so6882331lfn.11 for ; Mon, 06 May 2019 03:11:17 -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=2SDOjFeAWhmR9N+rjggxfBl+WrRtYrsCt6p5fnNWzQs=; b=VzYsMVTHwh3zhxRfo+k+nUnsfDZmQDgFPd+jA3Moaxvc+DPcDnrMPN6/HPUoht8X8A L1bV5DravP4DBgj9A8uNPW1sLIIs4/IkNsdszZZGWdtXxtU6hCuZjBFEuIwFpl6O6gRX FRtMOcJSws3IrCKjCkKI1/V3XXEeNR89Y2T1riS5InrRKqSvGbhBOcm3allrrTxDR/m8 0ECunZDnSdZYB70DVNNLCliG5cLV3BHSBcAVToKSObYf+SbW6hHqnDhDewIDaNIDfjcV MC6UW6DVq610B+BuiLXulcnF3wcXFqkzS89bA9kXBC1a62p2CBdrMkCunH4qPHj61W6m AayA== MIME-Version: 1.0 References: <7cb22a67-89e5-45d3-aef4-398311416140.bin.cheng@linux.alibaba.com> <20190417071001.GR21066@tucnak> <20190417112731.GV21066@tucnak> <7b52d36f-ab02-4370-9655-72a61cd227cd.bin.cheng@linux.alibaba.com> In-Reply-To: <7b52d36f-ab02-4370-9655-72a61cd227cd.bin.cheng@linux.alibaba.com> From: Richard Biener Date: Mon, 06 May 2019 10:11:00 -0000 Message-ID: Subject: Re: [PATCH PR90078]Capping comp_cost computation in ivopts To: "bin.cheng" Cc: Jakub Jelinek , GCC Patches Content-Type: text/plain; charset="UTF-8" X-IsSubscribed: yes X-SW-Source: 2019-05/txt/msg00188.txt.bz2 On Sun, May 5, 2019 at 8:03 AM bin.cheng wrote: > > > ------------------------------------------------------------------ > > Sender:Jakub Jelinek > > Sent At:2019 Apr. 17 (Wed.) 19:27 > > Recipient:Bin.Cheng > > Cc:bin.cheng ; GCC Patches > > Subject:Re: [PATCH PR90078]Capping comp_cost computation in ivopts > > > > > > On Wed, Apr 17, 2019 at 07:14:05PM +0800, Bin.Cheng wrote: > > > > As > > > > #define INFTY 10000000 > > > > what is the reason to keep the previous condition as well? > > > > I mean, if cost1.cost == INFTY or cost2.cost == INFTY, > > > > cost1.cost + cost2.cost >= INFTY too. > > > > Unless costs can go negative. > > > It's a bit complicated, but in general, costs can go negative. > > > > Ok, no objections from me then (but as I don't know anything about it, > > not an ack either; you are ivopts maintainer, so you don't need one). > > Hi, > The previous patch was reverted on GCC-9 because of PR90240. PR90240 is now > fixed by another patch. This is the updated patch for PR90078. It promotes type > of ivopts cost from int to int64_t, as well as change behavior of infinite_cost overflow > from saturation to assert. > Please note, implicit conversions are kept in cost computation as before without > introducing any narrowing. > > Bootstrap/test on x86_64 along with PR90240 patch. Is it OK? Do not include system headers in .c files, instead those need to be (and are already) included via system.h. /* The infinite cost. */ -#define INFTY 10000000 +#define INFTY 1000000000L do we actually need this? What happens on a ilp32 host? That is, I believe you can drop the 'L' (it fits into an int anyways) @@ -256,6 +259,7 @@ operator- (comp_cost cost1, comp_cost cost2) return infinite_cost; gcc_assert (!cost2.infinite_cost_p ()); + gcc_assert (cost1.cost - cost2.cost < infinite_cost.cost); cost1.cost -= cost2.cost; cost1.complexity -= cost2.complexity; probably a pre-existing issue, but we do not seem to handle underflow here in general, nor check that underflow doesn't get us below -INFTY. I guess we really don't want negative costs? That doesn't seem to be documented and I was also wondering why the cost isn't unsigned... @@ -638,7 +646,7 @@ struct iv_ca comp_cost cand_use_cost; /* Total cost of candidates. */ - unsigned cand_cost; + int64_t cand_cost; /* Number of times each invariant variable is used. */ unsigned *n_inv_var_uses; shows this "issue". Can't we use uint64_t throughout the patch? Otherwise this looks OK. Thanks, Richard. > Thanks, > bin > 2019-05-05 Bin Cheng > > PR tree-optimization/90078 > * tree-ssa-loop-ivopts.c (inttypes.h): Include new header file. > (INFTY): Increase the value for infinite cost. > (struct comp_cost): Promote type of members to int64_t. > (infinite_cost): Don't set complexity in initialization. > (comp_cost::operator +,-,+=,-+,/=,*=): Assert when cost computation > overflows to infinite_cost. > (adjust_setup_cost): Promote type of parameter and cost computation > to int64_t. > (struct ainc_cost_data, struct iv_ca): Promote type of member to > int64_t. > (get_scaled_computation_cost_at, determine_iv_cost): Promote type of > cost computation to int64_t. > (determine_group_iv_costs, iv_ca_dump, find_optimal_iv_set): Use > int64_t's format specifier in dump. > > 2018-05-05 Bin Cheng > > PR tree-optimization/90078 > * g++.dg/tree-ssa/pr90078.C: New test.