From patchwork Fri Apr 14 17:30:37 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jeff Law X-Patchwork-Id: 750905 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 3w4Pns4fTQz9s89 for ; Sat, 15 Apr 2017 03:30:56 +1000 (AEST) Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.b="csd/7jLM"; dkim-atps=neutral DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender :subject:to:references:from:message-id:date:mime-version :in-reply-to:content-type; q=dns; s=default; b=KBddSaoKpDIYWLFlE OteirtdxglrwlAjZTgdGdTweUMmIXAHQsYVIb33+aozY7Hj0vUgdn9QPG3AuaOvN J2vxNPL9RXw4lpdUWdWOIWmMT7WVe+i/bFy9nC1B6wDMxgcd7FmYR/7jYwlGGxGB 82KJ1p4pejYlSHKkw5HIqzwPNY= 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 :subject:to:references:from:message-id:date:mime-version :in-reply-to:content-type; s=default; bh=U1iQvBkhge9HoWYSO+y3SGM rO3I=; b=csd/7jLMDyReLekYCsqfxLg6vnBDaZv+OscC1HMLFXgqd6psSjtW0PX IhBvGt1H6OsGNPto+43H8pdQaIU+MGcuLezpP2lcLdmxwwkIOM5L0lyisE58psvb J+rkOG15AVTedMaenD9+2/oDc9d0uX2HOge3eWJlw4O2kbeJfwt0= Received: (qmail 38395 invoked by alias); 14 Apr 2017 17:30: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 38334 invoked by uid 89); 14 Apr 2017 17:30:39 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-25.9 required=5.0 tests=BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, KAM_LAZY_DOMAIN_SECURITY, RP_MATCHES_RCVD, SPF_HELO_PASS autolearn=ham version=3.3.2 spammy=ports 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; Fri, 14 Apr 2017 17:30:38 +0000 Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.phx2.redhat.com [10.5.11.14]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 359EE83F3F; Fri, 14 Apr 2017 17:30:38 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com 359EE83F3F Authentication-Results: ext-mx03.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx03.extmail.prod.ext.phx2.redhat.com; spf=pass smtp.mailfrom=law@redhat.com DKIM-Filter: OpenDKIM Filter v2.11.0 mx1.redhat.com 359EE83F3F Received: from localhost.localdomain (ovpn-116-10.phx2.redhat.com [10.3.116.10]) by smtp.corp.redhat.com (Postfix) with ESMTP id A88E517C4C; Fri, 14 Apr 2017 17:30:37 +0000 (UTC) Subject: Re: [PATCH] Fix newlib build failure for mips16 To: gcc-patches , rdsandiford@googlemail.com References: <8ae83192-929a-a677-639e-100e036672a0@redhat.com> <87efwvw7vw.fsf@googlemail.com> <46ac2419-b3cb-6895-6ae6-5d9d0cff3aa8@redhat.com> <87y3v3ufc9.fsf@googlemail.com> From: Jeff Law Message-ID: <045ca256-da1d-56a0-f97d-0063b1ef4b36@redhat.com> Date: Fri, 14 Apr 2017 11:30:37 -0600 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.8.0 MIME-Version: 1.0 In-Reply-To: <87y3v3ufc9.fsf@googlemail.com> X-IsSubscribed: yes On 04/14/2017 10:24 AM, Richard Sandiford wrote: > Jeff Law writes: >> On 04/14/2017 05:22 AM, Richard Sandiford wrote: >>> Jeff Law writes: >>>> The mips64vr-elf target will fail building newlib, particularly the >>>> mips16 newlib as we emit bogus assembly code. >>>> >>>> In particular the compiler will emit something like >>>> >>>> lwu $2,0($sp) >>>> >>>> That's invalid in mips16 mode AFAICT. >>>> >>>> That's emitted by the extendsidi pattern. It's a case where the operand >>>> predicates are looser that the constraints. The code we get out of >>>> reload is fine, but hard register propagation substitutes sp for a >>>> (valid mips16) hard register that as the same value. Since hard >>>> register propagation tests predicates, not constraints, the substitution >>>> is successful and the bogus code is generated. >>> >>> Isn't that the bug though? Post-reload passes must test the constraints >>> as well as the predicates, to make sure that the change aligns with >>> one of the available alternatives. >> I thought that as well and was quite surprised to see regcprop not honor >> that. >> >> regcprop uses the validate_change interface, which normally checks >> contraints -- except for MEMs which is just checks for validity via >> memory_address_addr_space_p. Thus inside a MEM it'll allow any change >> that is recognized as valid according to the legitimate_address hooks. >> >> I would claim that is fundamentally broken, but trying to change that at >> this stage is, IMHO, unwise. I think we should seriously consider doing >> something different for stage1. For example, after validating the MEM >> using the legitimate address hooks, call insn_invalid_p as well to >> verify constraints. > > I think it only does that if the "containing object" that you're > validating is a MEM. If the object you're validating is an insn > (which it always is for regcprop) then normal constrain_operands > does happen. > >>> Adding code to do that to individual MIPS patterns feels like a hack to me. >>> The pass should be doing it itself. >> Agreed. It's a hack. But it was the best I could see to do at this stage. > > Been looking at it a bit more, and I think the problem is that we're > somehow ending up with a second stack pointer rtx, distinct from > stack_pointer_rtx. And then I remembered that this had been discussed > before, see the tail end of: > > https://gcc.gnu.org/ml/gcc-patches/2016-01/msg02362.html > > I'd be happier with the mips_stack_address_p change described there, > although it still seems like a hack. Here's what I'm going to spin in my testers over the weekend. It reverts my mips.md change and instead rejects making a copy of the stack pointer. Jeff diff --git a/gcc/config/mips/mips.md b/gcc/config/mips/mips.md index dd5e1e7..7acf00d 100644 --- a/gcc/config/mips/mips.md +++ b/gcc/config/mips/mips.md @@ -3493,10 +3493,7 @@ (define_insn_and_split "*zero_extendsidi2" [(set (match_operand:DI 0 "register_operand" "=d,d") (zero_extend:DI (match_operand:SI 1 "nonimmediate_operand" "d,W")))] - "TARGET_64BIT && !ISA_HAS_EXT_INS - && !(TARGET_MIPS16 - && MEM_P (operands[1]) - && reg_mentioned_p (stack_pointer_rtx, operands[1]))" + "TARGET_64BIT && !ISA_HAS_EXT_INS" "@ # lwu\t%0,%1" @@ -3512,10 +3509,7 @@ (define_insn "*zero_extendsidi2_dext" [(set (match_operand:DI 0 "register_operand" "=d,d") (zero_extend:DI (match_operand:SI 1 "nonimmediate_operand" "d,W")))] - "TARGET_64BIT && ISA_HAS_EXT_INS - && !(TARGET_MIPS16 - && MEM_P (operands[1]) - && reg_mentioned_p (stack_pointer_rtx, operands[1]))" + "TARGET_64BIT && ISA_HAS_EXT_INS" "@ dext\t%0,%1,0,32 lwu\t%0,%1" diff --git a/gcc/regcprop.c b/gcc/regcprop.c index ddc6252..367d85a 100644 --- a/gcc/regcprop.c +++ b/gcc/regcprop.c @@ -396,6 +396,13 @@ maybe_mode_change (machine_mode orig_mode, machine_mode copy_mode, && GET_MODE_SIZE (copy_mode) < GET_MODE_SIZE (new_mode)) return NULL_RTX; + /* Avoid creating multiple copies of the stack pointer. Some ports + assume there is one and only one stack pointer. + + It's unclear if we need to do the same for other special registers. */ + if (regno == STACK_POINTER_REGNUM) + return NULL_RTX; + if (orig_mode == new_mode) return gen_raw_REG (new_mode, regno); else if (mode_change_ok (orig_mode, new_mode, regno))