From patchwork Wed Aug 27 10:35:38 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Bin.Cheng" X-Patchwork-Id: 383419 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)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 8A92514009B for ; Wed, 27 Aug 2014 20:38:26 +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=eeeF9hub95Isdi5W31 7LatZsN0mRcaQrzfgnf8GqyctZrYdbFhd0CW3i2SU1y7BjtNttryI7MCuDzVmaT+ pmCj0i0GkubwQkinwB1F+bhC3v3u0dRiMZ4lm/JI8AztlsCRmhq2D6Pd2FhnXc+a dm0rNeMr2PCIiTHiePBkJiquo= 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=kIyL1DRkHKNz8SMhzjHiubhu 0E8=; b=opii2PcW17Tx7UfZsBpWmtWddbcso718Lvs0uz+4IW28yLsfV6wZRtrP GdGOGPpwffRFcIB95FZcXY8mRfuJVNO08ThXIDPd1ThhKAYHfezuXbadMeI5eFQy U32nfFb+PgdGqVXk5Mic0G62vULMSwqpObdGd8wmSG+kpJqC2Fw= Received: (qmail 15424 invoked by alias); 27 Aug 2014 10:35:42 -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 15398 invoked by uid 89); 27 Aug 2014 10:35:41 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.2 required=5.0 tests=AWL, BAYES_00, FREEMAIL_FROM, RCVD_IN_DNSWL_LOW, SPF_PASS autolearn=ham version=3.3.2 X-HELO: mail-qa0-f43.google.com Received: from mail-qa0-f43.google.com (HELO mail-qa0-f43.google.com) (209.85.216.43) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-SHA encrypted) ESMTPS; Wed, 27 Aug 2014 10:35:40 +0000 Received: by mail-qa0-f43.google.com with SMTP id w8so10974qac.2 for ; Wed, 27 Aug 2014 03:35:38 -0700 (PDT) MIME-Version: 1.0 X-Received: by 10.140.106.225 with SMTP id e88mr52327778qgf.20.1409135738157; Wed, 27 Aug 2014 03:35:38 -0700 (PDT) Received: by 10.140.18.173 with HTTP; Wed, 27 Aug 2014 03:35:38 -0700 (PDT) In-Reply-To: References: <000001cfc1ce$5196ba10$f4c42e30$@arm.com> Date: Wed, 27 Aug 2014 18:35:38 +0800 Message-ID: Subject: Re: [PATCH GCC]Cleanup interface of iv_ca_add_use and the calls to it From: "Bin.Cheng" To: Richard Biener Cc: Bin Cheng , GCC Patches , Zdenek Dvorak X-IsSubscribed: yes On Wed, Aug 27, 2014 at 4:23 PM, Richard Biener wrote: > On Wed, Aug 27, 2014 at 10:10 AM, Bin Cheng wrote: >> Hi, >> As I analyzed in bug pr62178, current candidate selecting algorithm can't >> find out the optimal solution in some scenarios. I am trying to improve it >> but before that, I need to clean up the interface of iv_ca_add_use and the >> calls to it. The two calls to the function are controlled by a boolean >> argument, and the second call is always fired if the first one doesn't give >> any result. This patch encapsulates logic of the two calls into function >> iv_ca_add_use and cleanups the interface. >> Another change is remove the check in below code. >> >> gcc_assert (ivs->upto >= use->id); >> >> if (ivs->upto == use->id) >> { >> ivs->upto++; >> ivs->bad_uses++; >> } >> >> It can be proved in an inductive approach that ivs->up_to always equals to >> use->id at the position. >> >> This patch does not change code logic at all, anyway, it passes bootstrap >> and regtest on x86_64/x86. So is it OK? > > Ok (I suppose you checked that it really generates the same code on > a set of files?) Yes, I compared object files in stage3 gcc directory, only miscellaneous checksum files and tree-ssa-loop-ivopts.o itself are different. I suppose checksum files are trivial, right? Here attached the right patch, the old one missed one local variable declaration. I will commit in 24 hours if there is no other objection. Thanks, bin Index: gcc/tree-ssa-loop-ivopts.c =================================================================== --- gcc/tree-ssa-loop-ivopts.c (revision 214413) +++ gcc/tree-ssa-loop-ivopts.c (working copy) @@ -5415,36 +5415,40 @@ iv_ca_set_cp (struct ivopts_data *data, struct iv_ } /* Extend set IVS by expressing USE by some of the candidates in it - if possible. All important candidates will be considered - if IMPORTANT_CANDIDATES is true. */ + if possible. Consider all important candidates if candidates in + set IVS don't give any result. */ static void iv_ca_add_use (struct ivopts_data *data, struct iv_ca *ivs, - struct iv_use *use, bool important_candidates) + struct iv_use *use) { struct cost_pair *best_cp = NULL, *cp; bitmap_iterator bi; - bitmap cands; unsigned i; + struct iv_cand *cand; gcc_assert (ivs->upto >= use->id); + ivs->upto++; + ivs->bad_uses++; - if (ivs->upto == use->id) + EXECUTE_IF_SET_IN_BITMAP (ivs->cands, 0, i, bi) { - ivs->upto++; - ivs->bad_uses++; - } - - cands = (important_candidates ? data->important_candidates : ivs->cands); - EXECUTE_IF_SET_IN_BITMAP (cands, 0, i, bi) - { - struct iv_cand *cand = iv_cand (data, i); - + cand = iv_cand (data, i); cp = get_use_iv_cost (data, use, cand); - if (cheaper_cost_pair (cp, best_cp)) best_cp = cp; } + + if (best_cp == NULL) + { + EXECUTE_IF_SET_IN_BITMAP (data->important_candidates, 0, i, bi) + { + cand = iv_cand (data, i); + cp = get_use_iv_cost (data, use, cand); + if (cheaper_cost_pair (cp, best_cp)) + best_cp = cp; + } + } iv_ca_set_cp (data, ivs, use, best_cp); } @@ -5878,18 +5882,9 @@ try_add_cand_for (struct ivopts_data *data, struct struct iv_ca_delta *best_delta = NULL, *act_delta; struct cost_pair *cp; - iv_ca_add_use (data, ivs, use, false); + iv_ca_add_use (data, ivs, use); best_cost = iv_ca_cost (ivs); - cp = iv_ca_cand_for_use (ivs, use); - if (!cp) - { - ivs->upto--; - ivs->bad_uses--; - iv_ca_add_use (data, ivs, use, true); - best_cost = iv_ca_cost (ivs); - cp = iv_ca_cand_for_use (ivs, use); - } if (cp) { best_delta = iv_ca_delta_add (use, NULL, cp, NULL);