From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 55700 invoked by alias); 6 Nov 2018 20:28:07 -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 55636 invoked by uid 89); 6 Nov 2018 20:28:04 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-11.4 required=5.0 tests=BAYES_00,GIT_PATCH_2,GIT_PATCH_3,KAM_NUMSUBJECT,KAM_SHORT,SPF_HELO_PASS autolearn=ham version=3.3.2 spammy=Hx-languages-length:3131, absolutely, thru, constraint 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 ESMTP; Tue, 06 Nov 2018 20:28:02 +0000 Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.phx2.redhat.com [10.5.11.16]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 72D9881F0E; Tue, 6 Nov 2018 20:28:01 +0000 (UTC) Received: from localhost.localdomain (ovpn-112-42.rdu2.redhat.com [10.10.112.42]) by smtp.corp.redhat.com (Postfix) with ESMTP id 0EBB11750D; Tue, 6 Nov 2018 20:27:59 +0000 (UTC) Subject: Re: [RFC][PATCH LRA] WIP patch to fix one part of PR87507 To: Peter Bergner Cc: Vladimir N Makarov , Segher Boessenkool , GCC Patches References: <2c69adfc-b9db-687d-3a32-08f2efdde647@linux.ibm.com> <5391dd89-e511-76e8-a22a-0d3692b44d19@redhat.com> <927deb43-5e00-5b9c-91f6-3bc17dd613ef@linux.ibm.com> <8e712264-7ffd-5715-9d4a-35be74344e5a@linux.ibm.com> From: Jeff Law Openpgp: preference=signencrypt Message-ID: <2c42a37e-3f96-02e4-1c72-4c3712872f1a@redhat.com> Date: Tue, 06 Nov 2018 20:28:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.2.1 MIME-Version: 1.0 In-Reply-To: <8e712264-7ffd-5715-9d4a-35be74344e5a@linux.ibm.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-IsSubscribed: yes X-SW-Source: 2018-11/txt/msg00386.txt.bz2 On 10/27/18 10:24 AM, Peter Bergner wrote: > On 10/22/18 6:45 PM, Peter Bergner wrote: >> Bah, my bootstrap failed and I need to make a small change. Let me do that >> and verify my bootstraps get all the way thru before I give you the updated >> patch. Sorry. > Ok, the following updated patch survives bootstrap and regtesting on > powerpc64le-linux, x86_64-linux and s390x-linux with no regressions. > Changes from the previous patch is that checking for illegal hard register > usage in inline asm statements has been moved to expand time. Secondly, the > lra constraints code now checks for both HARD_REGISTER_P and REG_USERVAR_P. > This was because I was seeing combine forcing hard regs into a pattern > (not from inline asm) which lra needed to spill to match constraints, > which it should be able to do. > > Jeff, can you give this a try on your testers to see how it behaves on > the other arches that were having problems? > > Peter > > PR rtl-optimization/87600 > * cfgexpand.c (expand_asm_stmt): Catch illegal asm constraint usage. > * lra-constraints.c (process_alt_operands): Skip illegal hard > register usage. Prefer reloading non hard register operands. > > Index: gcc/lra-constraints.c > =================================================================== > --- gcc/lra-constraints.c (revision 265402) > +++ gcc/lra-constraints.c (working copy) > @@ -2146,9 +2146,30 @@ process_alt_operands (int only_alternati > } > else > { > - /* Operands don't match. Both operands must > - allow a reload register, otherwise we > - cannot make them match. */ > + /* Operands don't match. If the operands are > + different user defined explicit hard registers, > + then we cannot make them match. */ > + if ((REG_P (*curr_id->operand_loc[nop]) > + || SUBREG_P (*curr_id->operand_loc[nop])) > + && (REG_P (*curr_id->operand_loc[m]) > + || SUBREG_P (*curr_id->operand_loc[m]))) > + { > + rtx nop_reg = *curr_id->operand_loc[nop]; > + if (SUBREG_P (nop_reg)) > + nop_reg = SUBREG_REG (nop_reg); > + rtx m_reg = *curr_id->operand_loc[m]; > + if (SUBREG_P (m_reg)) > + m_reg = SUBREG_REG (m_reg); So the one worry I have/had in this code is nested subregs. My recollection is they do happen in rare cases. But I can also find a reference where Jim W. has indicated they're invalid (and I absolutely trust Jim on this kind of historical RTL stuff). https://gcc.gnu.org/ml/gcc/2005-04/msg01173.html Reviewing cases where we've written the stripping as a loop, several are stripping subregs as well as extractions. And I can certainly believe that we could have an RTX with some combination of subregs and extractions. The exception to that pattern would be reload which has subreg stripping loops that only strip the subregs. So, after all that, I think we're OK. It might make sense to verify we don't have nested subregs in the IL verifiers. Bonus points if you add that checking. So ideally we'd have some tests which tickle this code, even if they're target specific. So I'm OK with this patch and also OK with adding tests independently as a follow-up. jeff