From patchwork Thu Aug 6 23:59:09 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Hans-Peter Nilsson X-Patchwork-Id: 1342086 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=gcc.gnu.org (client-ip=8.43.85.97; helo=sourceware.org; envelope-from=gcc-patches-bounces@gcc.gnu.org; receiver=) Authentication-Results: ozlabs.org; dmarc=none (p=none dis=none) header.from=bitrange.com Received: from sourceware.org (server2.sourceware.org [8.43.85.97]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 4BN57R6300z9sPC for ; Fri, 7 Aug 2020 09:59:15 +1000 (AEST) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 33A7C3857037; Thu, 6 Aug 2020 23:59:13 +0000 (GMT) X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from arjuna.pair.com (arjuna.pair.com [209.68.5.131]) by sourceware.org (Postfix) with ESMTPS id 1D85D3857C45 for ; Thu, 6 Aug 2020 23:59:10 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 1D85D3857C45 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=bitrange.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=hp@bitrange.com Received: by arjuna.pair.com (Postfix, from userid 3006) id 8D6658A0BF; Thu, 6 Aug 2020 19:59:09 -0400 (EDT) Received: from localhost (localhost [127.0.0.1]) by arjuna.pair.com (Postfix) with ESMTP id 8ADEC8A099 for ; Thu, 6 Aug 2020 19:59:09 -0400 (EDT) Date: Thu, 6 Aug 2020 19:59:09 -0400 (EDT) From: Hans-Peter Nilsson X-X-Sender: hp@arjuna.pair.com To: gcc-patches@gcc.gnu.org Subject: mmix: fix gcc.dg/loop-9.c by more accurate move insns Message-ID: User-Agent: Alpine 2.20.16 (BSF 172 2016-09-29) MIME-Version: 1.0 X-Spam-Status: No, score=-4.6 required=5.0 tests=BAYES_00, KAM_DMARC_STATUS, RCVD_IN_DNSWL_LOW, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: gcc-patches-bounces@gcc.gnu.org Sender: "Gcc-patches" Committed. It looks like gcc.dg/loop-9.c kind-of works as sentinel for sane move-instruction generation for a port. Looking at the FAIL: gcc.dg/loop-9.c scan-rtl-dump loop2_invariant "Decided" FAIL: gcc.dg/loop-9.c scan-rtl-dump loop2_invariant "without introducing a new temporary register" it seems the problem is that in the loop: for (i = 0; i < 100; i++) a[i] = 18.4242; the move insn corresponding to "a[i] = 18.4242" happens to be generated as a move of a constant to a memory address, using no registers except for the address (edited): (insn 9 8 10 3 (set (mem:DF (reg:DI 269 [ ivtmp::9 ])) (const_double:DF 1.84241999e+1)) "x/loop-9.c":9:10 6 {movdf}) To wit, at the loop2 pass there's no register-initialization to move out of the loop! The insn above isn't accurate and has to be fixed up at register allocation time to make constraints match. While there are insns to set memory to constant in MMIX, that's limited to 64-bit moves corresponding to the integer bit-patterns for 0..255, and 18.4242 isn't one of them. (Only 0.0 matches; the bit-patterns for 0..255 would IIUC be interpreted as denormal floating-point numbers a.k.a. subnormal numbers and don't seem worthwhile to handle.) The fault is with the port, for not requiring a register for an operand that actually requires an intermediate register, in order to enable pre-register-allocation passes to do their job. The movdf pattern (actually, all MMIX movM), only required the destination to be a non-immediate operand and the source to be a general_operand, i.e. anything-to-anything. Better force the source to be a register, when asked to generate such a move insn. Also, make operands stay sane by using the matching insn condition to require one of the operands to be a register pre-register-allocation (for sake of combine-like passes that cook up "simplified" insns, possibly losing the use of a register). Looking no deeper than at the results of test-runs with different variants, I see that the latter "safety latch" has no effect on the test-results (at 919c9d4bd3db7da0), but it just feels like the right thing to do. Similarly, there's no effect on test-suite results, to do the same not just for movdf but for all moves. gcc: * config/mmix/mmix.md (MM): New mode_iterator. ("mov"): New expander to expand for all MM-modes. ("*movqi_expanded", "*movhi_expanded", "*movsi_expanded") ("*movsf_expanded", "*movdf_expanded"): Rename from the corresponding mov named pattern. Add to the condition that either operand must be a register_operand. ("*movdi_expanded"): Similar, but also allow STCO in the condition. --- gcc/gcc/config/mmix/mmix.md.orig Mon Jan 13 22:30:46 2020 +++ gcc/gcc/config/mmix/mmix.md Wed Aug 5 02:55:54 2020 @@ -38,6 +38,8 @@ (MMIX_rR_REGNUM 260) (MMIX_fp_rO_OFFSET -24)] ) + +(define_mode_iterator MM [QI HI SI DI SF DF]) ;; Operand and operator predicates. @@ -46,10 +48,25 @@ ;; FIXME: Can we remove the reg-to-reg for smaller modes? Shouldn't they ;; be synthesized ok? -(define_insn "movqi" +(define_expand "mov" + [(set (match_operand:MM 0 "nonimmediate_operand") + (match_operand:MM 1 "general_operand"))] + "" +{ + /* Help pre-register-allocation to use at least one register in a move. + FIXME: support STCO also for DFmode (storing 0.0). */ + if (!REG_P (operands[0]) && !REG_P (operands[1]) + && (mode != DImode + || !memory_operand (operands[0], DImode) + || !satisfies_constraint_I (operands[1]))) + operands[1] = force_reg (mode, operands[1]); +}) + +(define_insn "*movqi_expanded" [(set (match_operand:QI 0 "nonimmediate_operand" "=r,r ,r,x ,r,r,m,??r") (match_operand:QI 1 "general_operand" "r,LS,K,rI,x,m,r,n"))] - "" + "register_operand (operands[0], QImode) + || register_operand (operands[1], QImode)" "@ SET %0,%1 %s1 %0,%v1 @@ -60,10 +77,11 @@ STBU %1,%0 %r0%I1") -(define_insn "movhi" +(define_insn "*movhi_expanded" [(set (match_operand:HI 0 "nonimmediate_operand" "=r,r ,r ,x,r,r,m,??r") (match_operand:HI 1 "general_operand" "r,LS,K,r,x,m,r,n"))] - "" + "register_operand (operands[0], HImode) + || register_operand (operands[1], HImode)" "@ SET %0,%1 %s1 %0,%v1 @@ -75,10 +93,11 @@ %r0%I1") ;; gcc.c-torture/compile/920428-2.c fails if there's no "n". -(define_insn "movsi" +(define_insn "*movsi_expanded" [(set (match_operand:SI 0 "nonimmediate_operand" "=r,r ,r,x,r,r,m,??r") (match_operand:SI 1 "general_operand" "r,LS,K,r,x,m,r,n"))] - "" + "register_operand (operands[0], SImode) + || register_operand (operands[1], SImode)" "@ SET %0,%1 %s1 %0,%v1 @@ -90,10 +109,13 @@ %r0%I1") ;; We assume all "s" are addresses. Does that hold? -(define_insn "movdi" +(define_insn "*movdi_expanded" [(set (match_operand:DI 0 "nonimmediate_operand" "=r,r ,r,x,r,m,r,m,r,r,??r") (match_operand:DI 1 "general_operand" "r,LS,K,r,x,I,m,r,R,s,n"))] - "" + "register_operand (operands[0], DImode) + || register_operand (operands[1], DImode) + || (memory_operand (operands[0], DImode) + && satisfies_constraint_I (operands[1]))" "@ SET %0,%1 %s1 %0,%v1 @@ -109,10 +131,11 @@ ;; Note that we move around the float as a collection of bits; no ;; conversion to double. -(define_insn "movsf" +(define_insn "*movsf_expanded" [(set (match_operand:SF 0 "nonimmediate_operand" "=r,r,x,r,r,m,??r") (match_operand:SF 1 "general_operand" "r,G,r,x,m,r,F"))] - "" + "register_operand (operands[0], SFmode) + || register_operand (operands[1], SFmode)" "@ SET %0,%1 SETL %0,0 @@ -122,10 +145,11 @@ STTU %1,%0 %r0%I1") -(define_insn "movdf" +(define_insn "*movdf_expanded" [(set (match_operand:DF 0 "nonimmediate_operand" "=r,r,x,r,r,m,??r") (match_operand:DF 1 "general_operand" "r,G,r,x,m,r,F"))] - "" + "register_operand (operands[0], DFmode) + || register_operand (operands[1], DFmode)" "@ SET %0,%1 SETL %0,0