From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 42812 invoked by alias); 31 May 2017 14:04:40 -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 42796 invoked by uid 89); 31 May 2017 14:04:39 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-12.0 required=5.0 tests=AWL,BAYES_00,GIT_PATCH_2,GIT_PATCH_3,RCVD_IN_DNSWL_LOW autolearn=ham version=3.3.2 spammy= X-HELO: mo4-p00-ob.smtp.rzone.de Received: from mo4-p00-ob.smtp.rzone.de (HELO mo4-p00-ob.smtp.rzone.de) (81.169.146.219) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 31 May 2017 14:04:37 +0000 X-RZG-AUTH: :LXoWVUeid/7A29J/hMvvT3ol15ykJcYwR/bcHRirORRW3yMcVao= X-RZG-CLASS-ID: mo00 Received: from [192.168.0.123] (mail.hightec-rt.com [213.135.1.215]) by smtp.strato.de (RZmta 40.7 DYNA|AUTH) with ESMTPSA id a00515t4VE4XACh (using TLSv1.2 with cipher ECDHE-RSA-AES256-SHA (curve secp521r1 with 521 ECDH bits, eq. 15360 bits RSA)) (Client did not present a certificate); Wed, 31 May 2017 16:04:33 +0200 (CEST) Subject: Re: [PATCH] Optimize divmod expansion (PR middle-end/79665) To: Jakub Jelinek References: <20170222214046.GA1849@tucnak> <99048e22-aedc-df95-f1fe-dc1eaffd58b1@gjlay.de> <20170531081554.GP24023@tucnak> <20170531090041.GR24023@tucnak> Cc: Jeff Law , gcc-patches@gcc.gnu.org From: Georg-Johann Lay Message-ID: <91f8b951-3a65-cc31-2720-7f86922458e9@gjlay.de> Date: Wed, 31 May 2017 14:04:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.7.1 MIME-Version: 1.0 In-Reply-To: <20170531090041.GR24023@tucnak> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit X-IsSubscribed: yes X-SW-Source: 2017-05/txt/msg02364.txt.bz2 On 31.05.2017 11:00, Jakub Jelinek wrote: > On Wed, May 31, 2017 at 10:48:07AM +0200, Georg-Johann Lay wrote: >>>> because divmod in not a single_set: >>>> (gdb) p seq >>>> $10 = (const rtx_insn *) 0x7ffff730d500 >>>> (gdb) pr >>>> warning: Expression is not an assignment (and might have no effect) >>>> (insn 14 13 0 (parallel [ >>>> (set (reg:HI 52) >>>> (div:HI (reg:HI 47) >>>> (reg:HI 54))) >>>> (set (reg:HI 53) >>>> (mod:HI (reg:HI 47) >>>> (reg:HI 54))) >>>> (clobber (reg:QI 21 r21)) >>>> (clobber (reg:HI 22 r22)) >>>> (clobber (reg:HI 24 r24)) >>>> (clobber (reg:HI 26 r26)) >>>> ]) "scale.c":7 -1 >>>> (nil)) >>>> (gdb) >>>> >>>> Hence the divmod appears to be much less expensive than the unsigned >>>> variant that computed the costs for mult_highpart. >>> >>> Then you should fix the cost computation - be able to use a target hook >>> on insns that are not a single set or something similar. >> >> Are you saying that cost computation in GCC is fundamentally flawed >> for anything that it not a single_set? > > The division/modulo optimization I've added as well as many other spots > in GCC rely on reasonable cost, just grep e.g. all places that call > seq_cost. So, if it returns something that is a very wrong estimate, > it won't affect just that single optimization, but all others. Therefore, > you should fix the cost computation, rather than disabling all the places > that use the costs. Many targets have instructions with multiple sets, I didn't intend to disable anything... Would the following addition be in order? gcc/ PR middle-end/80929 * rtlanal.c (seq_cost) [PARALLEL]: Get cost from insn_rtx_cost instead of assuming cost of 1. Index: rtlanal.c =================================================================== --- rtlanal.c (revision 248737) +++ rtlanal.c (working copy) @@ -5300,6 +5300,8 @@ seq_cost (const rtx_insn *seq, bool spee set = single_set (seq); if (set) cost += set_rtx_cost (set, speed); + else if (PARALLEL == GET_CODE (PATTERN (seq))) + cost += insn_rtx_cost (PATTERN (seq), speed); else cost++; } > so I'm surprised assuming cost of 1 for them doesn't break many more things. Maybe because PARALLEL is not common, and when expand tests for costs of DIV or MOD, it passes respective RTXes to the RTL cost functions, and *not* what the target expands in divmod insns. > I think either we should have a separate target hook for multiple sets > instructions, or just call the targetm.rtx_costs on the PARALLEL in that > case and see if the targets compute something reasonable for it, otherwise > either use the cost of the first set, or maximum of all sets (that might be > best) or something similar. > > Jakub The patch uses whatever insn_rtx_costs comes up with. For PARALLEL, it's the cost of the 1st SET which is reasonable imo (at least for the divmod case). Johann