From patchwork Mon Mar 4 09:04:01 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Jakub Jelinek X-Patchwork-Id: 1051020 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (mailfrom) smtp.mailfrom=gcc.gnu.org (client-ip=209.132.180.131; helo=sourceware.org; envelope-from=gcc-patches-return-497288-incoming=patchwork.ozlabs.org@gcc.gnu.org; receiver=) Authentication-Results: ozlabs.org; dmarc=fail (p=none dis=none) header.from=redhat.com Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.b="EmCGcrvC"; dkim-atps=neutral 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 44CYxP6Zjpz9s47 for ; Mon, 4 Mar 2019 20:04:23 +1100 (AEDT) DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender:date :from:to:cc:subject:message-id:reply-to:references:mime-version :content-type:content-transfer-encoding:in-reply-to; q=dns; s= default; b=yDPUupNDEmno/RLeV2BJH1c31JGqFzmXX9A8n+hQAR3YUIur8LnL/ uJDLzclqXkkSXh292J/A/ieuI7GKBMTtv8w6uaM6hWvFfCIBkPdR6Ul0ali9+pCX XBcsZSAcwxFA67OhhtLdQbFu8xA0CmRtnRdAVrMqSUM6pXXlmt7apA= 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:date :from:to:cc:subject:message-id:reply-to:references:mime-version :content-type:content-transfer-encoding:in-reply-to; s=default; bh=+Mymlymt0/LddNU49y+CL39NXr0=; b=EmCGcrvC4mhjUgOm6jemKoApcx9L F27eWpq2UQgjPZKh7dPiWcmDYX7QI8y8GfvCdUwIcjHiCSIKIfncgTrUrQnv3mo8 JZ9cXb8pBTBGq8TI38jC4TUc+6AfCzQwMAH+M069DQ5Sg6zdq6nKMkQmyYT2MTsm C4Ntzszw1fDRhmY= Received: (qmail 114724 invoked by alias); 4 Mar 2019 09:04:15 -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 114704 invoked by uid 89); 4 Mar 2019 09:04:14 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-10.9 required=5.0 tests=BAYES_00, GIT_PATCH_2, GIT_PATCH_3, KAM_LAZY_DOMAIN_SECURITY, SPF_HELO_PASS autolearn=ham version=3.3.2 spammy=il, LI, match_operand, define_insn 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; Mon, 04 Mar 2019 09:04:09 +0000 Received: from smtp.corp.redhat.com (int-mx05.intmail.prod.int.phx2.redhat.com [10.5.11.15]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 0FE0A30821B3; Mon, 4 Mar 2019 09:04:08 +0000 (UTC) Received: from tucnak.zalov.cz (ovpn-117-64.ams2.redhat.com [10.36.117.64]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 9246F5D71C; Mon, 4 Mar 2019 09:04:07 +0000 (UTC) Received: from tucnak.zalov.cz (localhost [127.0.0.1]) by tucnak.zalov.cz (8.15.2/8.15.2) with ESMTP id x24945Vq031788; Mon, 4 Mar 2019 10:04:05 +0100 Received: (from jakub@localhost) by tucnak.zalov.cz (8.15.2/8.15.2/Submit) id x24941C8031787; Mon, 4 Mar 2019 10:04:01 +0100 Date: Mon, 4 Mar 2019 10:04:01 +0100 From: Jakub Jelinek To: Kyrylo Tkachov , Richard Earnshaw , Ramana Radhakrishnan Cc: Wilco Dijkstra , gcc-patches@gcc.gnu.org Subject: [PATCH] ARM cmpsi2_addneg fix follow-up (PR target/89506) Message-ID: <20190304090401.GG7611@tucnak> Reply-To: Jakub Jelinek References: <20190301150748.GQ7611@tucnak> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.10.1 (2018-07-13) X-IsSubscribed: yes On Fri, Mar 01, 2019 at 03:41:33PM +0000, Wilco Dijkstra wrote: > > and regtest revealed two code size > > regressions because of that.  Is -1 vs. 1 the only case of immediate > > valid for both "I" and "L" constraints where the former is longer than the > > latter? > > Yes -1 is the only case which can result in larger code on Thumb-2, so -1 should > be disallowed by the I constraint (or even better, the underlying query). That way > it will work correctly for all add/sub patterns, not just this one. So, over the weekend I've bootstrapped/regtested on armv7hl-linux-gnueabi following two possible follow-ups which handle the -1 and 1 cases right (prefer the instruction with #1 for thumb2), 0 and INT_MIN (use subs) and for others use subs if both constraints match, otherwise adds. The first one uses constraints and no C code in the output, I believe it is actually more expensive for compile time, because if one just reads what constrain_operands needs to do for another constraint, it is quite a lot. I've tried to at least not introduce new constraints for this, there is no constraint for number 1 (or for number -1). The Pu constraint is thumb2 only for numbers 1..8, and the alternative uses I constraint for the negation of it, i.e. -8..-1, only -1 from this is valid for I. If that matches, we emit adds with #1, otherwise just prefer subs over adds. The other swaps the alternatives similarly to the above, but for the special case of desirable adds with #1 uses C code instead of another alternative. Ok for trunk (which one)? Jakub 2019-03-04 Jakub Jelinek PR target/89506 * config/arm/arm.md (cmpsi2_addneg): Swap the alternatives, add another alternative with I constraint for operands[2] and Pu for operands[3] and emit adds in that case, don't use C code to emit the instruction. 2019-03-04 Jakub Jelinek PR target/89506 * config/arm/arm.md (cmpsi2_addneg): Swap the alternatives and use subs for the first alternative except when operands[3] is 1. --- gcc/config/arm/arm.md.jj 2019-03-02 09:04:25.550794239 +0100 +++ gcc/config/arm/arm.md 2019-03-02 09:41:03.501404694 +0100 @@ -857,27 +857,27 @@ (define_insn "*compare_negsi_si" (set_attr "type" "alus_sreg")] ) -;; This is the canonicalization of addsi3_compare0_for_combiner when the +;; This is the canonicalization of subsi3_compare when the ;; addend is a constant. (define_insn "cmpsi2_addneg" [(set (reg:CC CC_REGNUM) (compare:CC (match_operand:SI 1 "s_register_operand" "r,r") - (match_operand:SI 2 "arm_addimm_operand" "L,I"))) + (match_operand:SI 2 "arm_addimm_operand" "I,L"))) (set (match_operand:SI 0 "s_register_operand" "=r,r") (plus:SI (match_dup 1) - (match_operand:SI 3 "arm_addimm_operand" "I,L")))] + (match_operand:SI 3 "arm_addimm_operand" "L,I")))] "TARGET_32BIT && (INTVAL (operands[2]) == trunc_int_for_mode (-INTVAL (operands[3]), SImode))" { - /* For 0 and INT_MIN it is essential that we use subs, as adds - will result in different condition codes (like cmn rather than - like cmp). For other immediates, we should choose whatever - will have smaller encoding. */ - if (operands[2] == const0_rtx - || INTVAL (operands[2]) == -HOST_WIDE_INT_C (0x80000000) - || which_alternative == 1) + /* For 0 and INT_MIN it is essential that we use subs, as adds will result + in different condition codes (like cmn rather than like cmp), so that + alternative comes first. Both alternatives can match for any 0x??000000 + where except for 0 and INT_MIN it doesn't matter what we choose, and also + for -1 and 1 with TARGET_THUMB2, in that case prefer instruction with #1 + as it is shorter. */ + if (which_alternative == 0 && operands[3] != const1_rtx) return "subs%?\\t%0, %1, #%n3"; else return "adds%?\\t%0, %1, %3"; --- gcc/config/arm/arm.md.jj 2019-03-02 09:04:25.550794239 +0100 +++ gcc/config/arm/arm.md 2019-03-02 17:08:13.036725812 +0100 @@ -857,31 +857,31 @@ (define_insn "*compare_negsi_si" (set_attr "type" "alus_sreg")] ) -;; This is the canonicalization of addsi3_compare0_for_combiner when the +;; This is the canonicalization of subsi3_compare when the ;; addend is a constant. +;; For 0 and INT_MIN it is essential that we use subs, as adds will result +;; in different condition codes (like cmn rather than like cmp), so that +;; alternative comes first. Both I and L constraints can match for any +;; 0x??000000 where except for 0 and INT_MIN it doesn't matter what we choose, +;; and also for -1 and 1 with TARGET_THUMB2, in that case prefer instruction +;; with #1 as it is shorter. The first alternative will use adds ?, ?, #1 over +;; subs ?, ?, #-1, the second alternative will use subs for #0 or #2147483648 +;; or any other case where both I and L constraints match. (define_insn "cmpsi2_addneg" [(set (reg:CC CC_REGNUM) (compare:CC - (match_operand:SI 1 "s_register_operand" "r,r") - (match_operand:SI 2 "arm_addimm_operand" "L,I"))) - (set (match_operand:SI 0 "s_register_operand" "=r,r") + (match_operand:SI 1 "s_register_operand" "r,r,r") + (match_operand:SI 2 "arm_addimm_operand" "I,I,L"))) + (set (match_operand:SI 0 "s_register_operand" "=r,r,r") (plus:SI (match_dup 1) - (match_operand:SI 3 "arm_addimm_operand" "I,L")))] + (match_operand:SI 3 "arm_addimm_operand" "Pu,L,I")))] "TARGET_32BIT && (INTVAL (operands[2]) == trunc_int_for_mode (-INTVAL (operands[3]), SImode))" -{ - /* For 0 and INT_MIN it is essential that we use subs, as adds - will result in different condition codes (like cmn rather than - like cmp). For other immediates, we should choose whatever - will have smaller encoding. */ - if (operands[2] == const0_rtx - || INTVAL (operands[2]) == -HOST_WIDE_INT_C (0x80000000) - || which_alternative == 1) - return "subs%?\\t%0, %1, #%n3"; - else - return "adds%?\\t%0, %1, %3"; -} + "@ + adds%?\\t%0, %1, %3 + subs%?\\t%0, %1, #%n3 + adds%?\\t%0, %1, %3" [(set_attr "conds" "set") (set_attr "type" "alus_sreg")] )