From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 18094 invoked by alias); 11 Mar 2016 21:56:01 -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 18012 invoked by uid 89); 11 Mar 2016 21:56:00 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=0.8 required=5.0 tests=BAYES_50,RP_MATCHES_RCVD,SPF_HELO_PASS autolearn=ham version=3.3.2 spammy=reside, reg_renumber, blindly, disallow X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-GCM-SHA384 encrypted) ESMTPS; Fri, 11 Mar 2016 21:55:54 +0000 Received: from int-mx11.intmail.prod.int.phx2.redhat.com (int-mx11.intmail.prod.int.phx2.redhat.com [10.5.11.24]) by mx1.redhat.com (Postfix) with ESMTPS id AC11449DCB for ; Fri, 11 Mar 2016 21:55:53 +0000 (UTC) Received: from slagheap.utah.redhat.com (ovpn-113-182.phx2.redhat.com [10.3.113.182]) by int-mx11.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id u2BLtqTr021791; Fri, 11 Mar 2016 16:55:53 -0500 Subject: Re: LRA remat issue with hard regs (PR70123) To: Bernd Schmidt , GCC Patches , Vladimir Makarov References: <56E1B422.6020000@t-online.de> <56E1B8B0.4030105@redhat.com> Cc: Jakub Jelinek From: Jeff Law Message-ID: <56E33EE8.8060807@redhat.com> Date: Fri, 11 Mar 2016 21:56:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.6.0 MIME-Version: 1.0 In-Reply-To: <56E1B8B0.4030105@redhat.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit X-IsSubscribed: yes X-SW-Source: 2016-03/txt/msg00717.txt.bz2 On 03/10/2016 11:10 AM, Bernd Schmidt wrote: > When I submitted my previous lra-remat patch, I mentioned I had some > concerns about the way we dealt with register number comparisons, but I > didn't want to change things blindly without a testcase. PR70123 has now > provided such a testcase where we are trying to rematerialize a hard > register (r6). While scanning we encounter an instruction of the form > (set (reg 285) (reg 272)) > i.e. involving only pseudos, but reg_renumber[285] is r6. Since we only > compare register numbers, we do not notice that the hard reg is clobbered. > > The following patch modifies the function input_regno_present_p, and > also renames it so that its purpose is more obvious to someone familiar > with other parts of gcc. I've made it look at reg_renumber, and also try > to deal with multi-word hard registers properly. > > I'm not entirely sure this is a fully safe approach however, since I > can't yet answer the question of whether LRA could change another pseudo > to reside in hard register 6, thereby making the rematerialization > invalid after the fact. Therefore the patch also includes a change to > just disable candidates if they involve hard registers. I haven't > observed that making any difference in code generation (on x86_64), > beyond fixing the testcase on s390. > > Bootstrapped and tested on x86_64-linux; Jakub verified that the > testcase works afterwards. Ok for trunk and 5-branch, either for one or > for both parts? I'm hoping the testcase in gcc.dg/torture will get > exercised in the right way on s390, but I haven't run tests on that > machine. > > > Bernd > > remat-hardregs.diff > > > PR target/70123 > * lra-remat.c (operand_to_remat): Disallow hard regs in the value t > be rematerialized. > (reg_overlap_for_remat_p): Renamed from input_regno_present_p. > Arguments swapped. All callers changed. Take reg_renumber into > account, and Calculate and compare register ranges for hard regs. > > PR target/70123 > * gcc.dg/torture/pr70123.c: New test. OK. Like you I'm not sure if the operand_to_remat test is strictly necessary, but I can't see how it'll ever generate incorrect code. The change to reg_overlap_for_remat_p looks good and should be strictly an improvement from a correctness standpoint. I think both are OK for the trunk and the gcc-5 branch after a bit of soaking on the trunk. jeff