From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 52620 invoked by alias); 5 May 2019 03:23:06 -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 52603 invoked by uid 89); 5 May 2019 03:23:05 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-7.5 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_LOW,SPF_PASS autolearn=ham version=3.3.1 spammy=balance, vacation, useless, reviewed X-HELO: mx0a-001b2d01.pphosted.com Received: from mx0b-001b2d01.pphosted.com (HELO mx0a-001b2d01.pphosted.com) (148.163.158.5) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Sun, 05 May 2019 03:23:04 +0000 Received: from pps.filterd (m0098417.ppops.net [127.0.0.1]) by mx0a-001b2d01.pphosted.com (8.16.0.27/8.16.0.27) with SMTP id x453LTpU137732 for ; Sat, 4 May 2019 23:23:01 -0400 Received: from e06smtp05.uk.ibm.com (e06smtp05.uk.ibm.com [195.75.94.101]) by mx0a-001b2d01.pphosted.com with ESMTP id 2s9qtt82v6-1 (version=TLSv1.2 cipher=AES256-GCM-SHA384 bits=256 verify=NOT) for ; Sat, 04 May 2019 23:23:01 -0400 Received: from localhost by e06smtp05.uk.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Sun, 5 May 2019 04:22:59 +0100 Received: from b06cxnps3075.portsmouth.uk.ibm.com (9.149.109.195) by e06smtp05.uk.ibm.com (192.168.101.135) with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted; (version=TLSv1/SSLv3 cipher=AES256-GCM-SHA384 bits=256/256) Sun, 5 May 2019 04:22:57 +0100 Received: from d06av24.portsmouth.uk.ibm.com (mk.ibm.com [9.149.105.60]) by b06cxnps3075.portsmouth.uk.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id x453MubL59965596 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Sun, 5 May 2019 03:22:56 GMT Received: from d06av24.portsmouth.uk.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id 96C9642047; Sun, 5 May 2019 03:22:56 +0000 (GMT) Received: from d06av24.portsmouth.uk.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id 269E742041; Sun, 5 May 2019 03:22:54 +0000 (GMT) Received: from kewenlins-mbp.cn.ibm.com (unknown [9.200.147.179]) by d06av24.portsmouth.uk.ibm.com (Postfix) with ESMTP; Sun, 5 May 2019 03:22:53 +0000 (GMT) Subject: Re: [PATCH, RFC, rs6000] PR80791 Consider doloop in ivopts To: "Bin.Cheng" Cc: GCC Patches , "bin.cheng" , Segher Boessenkool , Bill Schmidt , Richard Guenther , Jakub Jelinek References: From: "Kewen.Lin" Date: Sun, 05 May 2019 03:23:00 -0000 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.14; rv:60.0) Gecko/20100101 Thunderbird/60.6.1 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit x-cbid: 19050503-0020-0000-0000-000003396415 X-IBM-AV-DETECTION: SAVI=unused REMOTE=unused XFE=unused x-cbparentid: 19050503-0021-0000-0000-0000218BF4B2 Message-Id: X-IsSubscribed: yes X-SW-Source: 2019-05/txt/msg00159.txt.bz2 Hi Bin, Sorry for late response (just back from vacation). Thanks very much for your comments. on 2019/4/27 上午11:20, Bin.Cheng wrote: > For such non-trivial patch, we can improve review process by splitting > to smaller patches which can be reviewed/approved independently. Good idea! I'll split it. > Below are some general comments specific to IVOPTs change. >> for (j = 0; j < group->vuses.length (); j++) >> { >> - rewrite_use_compare (data, group->vuses[j], cand); >> - update_stmt (group->vuses[j]->stmt); >> + rewrite_use_compare (data, group->vuses[j], cand); >> + update_stmt (group->vuses[j]->stmt); > Wrong formatting change? Also we can run contrib/check_GNU_style.sh > for coding style issues. > Good catch! >> + /* Some compare iv_use is probably useless once the doloop optimization >> + performs. */ >> + if (tailor_cmp_p) >> + tailor_cmp_uses (data); > Function tailor_cmp_uses sets iv_use->zero_cost_p under some > conditions. Once the flag is set, though the iv_use participates cost > computation in determine_group_iv_cost_cond, the result cost is > hard-wired to ZERO (which means cost computation for such iv_use is > waste of time). Yes, it can be improved by some early check and return. But it's still helpful to make it call with may_eliminate_iv. gcc.dg/no-strict-overflow-6.c is one example, with may_eliminate_iv consideration it exposes more opportunities for downstream optimization. > Also iv_use rewriting process is skipped for related > ivs preserved explicitly by preserve_ivs_for_use. > Note IVOPTs already adds candidate for original ivs. So why not > detecting doloop iv_use, adjust its cost with the corresponding > original iv candidate, then let the general cost model make the > decision? I believe this better fits existing infrastructure and > requires less change, for example, preserve_ivs_for_use won't be > necessary. I agree adjusting the cost of original iv candidate of the iv_use requires less change, but on one hand, I thought to remove interest cmp iv use or make it zero cost is close to the fact. Since it's eliminated eventually in doloop optimization, it should not considered in cost modeling. This way looks more exact. One the other hand, I assumed your suggestion is to increase the cost for the pair (original iv cand, cmp iv use), the increase cost seems to be a heuristic value? It seems it's possible to sacrifice performance for some worst cases? Although it's integrated to the existing framework, it probably introduces other cost adjust issues. For example, the original iv cand is inclined not to be selected after this change, but without considering the cmp iv use, it's better to be selected. It looks highly depend on the cost tuning? Does my understanding above make sense? But I will follow your suggestion to update and check the regression testing result etc. I just updated the code to increase the cost for the pair (original iv cand, cmp iv use) with 5 (to balance the original iv 4 vs other 5), the failure case in PR80791 was fixed. I assumed IP_ORIGINAL for cand->pos to check original iv cand is enough. > It has other advantages too, for example, 1) candidate of > original iv can be preferred for other iv_uses with doloop cost I think the patch way is not intrusive either. > tuning; 2) the doloop decision can still be canceled by cost model if > it's really not beneficial. With current patch, it can't be undo once > the decision is made (at very early stage in IVOPTs.). I can't really follow this. If it's predicted to be transformed to doloop, I think it should not be undoed any more, since it's useless to consider this cmp iv use. Whatever IVOPTS does, the comp at loop closing should not be changed (although possible to use other iv), right? Do I miss something? > Sorry didn't bring out this earlier, I only realized this after > reading your code. It doesn't matter indeed! Thanks again!