From patchwork Wed Nov 16 10:16:16 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Richard Sandiford X-Patchwork-Id: 695509 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 3tJgCc5r4fz9ryn for ; Wed, 16 Nov 2016 21:16:43 +1100 (AEDT) Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.b="qtvdynW9"; 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:from :to:cc:subject:references:date:in-reply-to:message-id :mime-version:content-type; q=dns; s=default; b=OGYNElWzGSO24D7o qtMRa28SYrUeT5aVd7DYUSM+Eb2Gnov8cgdenPzAydoiLkuD+UF+G7Dk6Rt0IuFG DiAZ4lncY1vbj+AUL1CjWkWl0YN5dOkLd1Awf3/S8iHGlFwCdIWJBlDswaTWB2DS VuYZoCAv2+BFlja4JDW5vqYrbRc= 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:from :to:cc:subject:references:date:in-reply-to:message-id :mime-version:content-type; s=default; bh=f+N/mbh3AYQ6lpU/H8KlWn EfrkQ=; b=qtvdynW9Z0OwFCYFK538SkxmKy5ZP+5nlXAWUPwVnFUoIs7+5h1nsw AZX177EGh1YlYjhN7SuXjA52LGWHtiaIC+mBJdYnj7S+Dlp0OPhik8OxFQOsRRZ8 A3biyvYOzYx7T1wIieWfMpmzjbaRGI3FJfMmSs2BW4ittXuMusHR8= Received: (qmail 11822 invoked by alias); 16 Nov 2016 10:16:33 -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 11801 invoked by uid 89); 16 Nov 2016 10:16:32 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-4.8 required=5.0 tests=BAYES_00, RP_MATCHES_RCVD autolearn=ham version=3.3.2 spammy=78666, our X-HELO: foss.arm.com Received: from foss.arm.com (HELO foss.arm.com) (217.140.101.70) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 16 Nov 2016 10:16:21 +0000 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.72.51.249]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 1DFCB16; Wed, 16 Nov 2016 02:16:20 -0800 (PST) Received: from localhost (e105548-lin.manchester.arm.com [10.45.32.67]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 7F8463F318; Wed, 16 Nov 2016 02:16:18 -0800 (PST) From: Richard Sandiford To: Segher Boessenkool Mail-Followup-To: Segher Boessenkool , , richard.sandiford@arm.com Cc: Subject: Re: An alternative fix for PR70944 References: <87d1hw53el.fsf@e105548-lin.cambridge.arm.com> <20161115131322.GF12325@gate.crashing.org> Date: Wed, 16 Nov 2016 10:16:16 +0000 In-Reply-To: <20161115131322.GF12325@gate.crashing.org> (Segher Boessenkool's message of "Tue, 15 Nov 2016 07:13:22 -0600") Message-ID: <8737irzq4v.fsf@e105548-lin.cambridge.arm.com> User-Agent: Gnus/5.130012 (Ma Gnus v0.12) Emacs/24.3 (gnu/linux) MIME-Version: 1.0 Segher Boessenkool writes: > On Tue, Nov 15, 2016 at 12:33:06PM +0000, Richard Sandiford wrote: >> The transformations made by make_compound_operation apply >> only to scalar integer modes. The fix for PR70944 had enforced >> that by returning early for vector modes at the top of the >> function. However, the function is supposed to be recursive, >> so we should continue to look at integer suboperands even if >> the outer operation is a vector one. >> >> This patch instead splits out the non-recursive parts >> of make_compound_operation into a subroutine and checks >> that the mode is a scalar integer before calling it. >> The patch was originally written to help with the later >> conversion to static type checking of mode classes, but it >> also happened to reenable optimisation of things like >> vec_duplicate operands. >> >> Note that the gen_lowparts in the PLUS and MINUS cases >> were redundant, since new_rtx already had mode "mode" >> at those points. >> >> Tested on aarch64-linux-gnu and x86_64-linux-gnu. OK to install? > > Yes, please do. You can use maybe_swap_commutative_operands in > change_zero_ext as well, perhaps in more places, do you want to > take a look? Ah, yeah. change_zero_ext was the only one I could see. Tested on aarch64-linux-gnu and x86_64-linux-gnu. OK to install? Thanks, Richard gcc/ 2016-11-15 Richard Sandiford Alan Hayward David Sherwood * combine.c (maybe_swap_commutative_operands): New function. (combine_simplify_rtx): Use it. (change_zero_ext): Likewise. (make_compound_operation_int): New function, split out of... (make_compound_operation): ...here. Use maybe_swap_commutative_operands for both. diff --git a/gcc/combine.c b/gcc/combine.c index 19ef52f..ca5ddae 100644 --- a/gcc/combine.c +++ b/gcc/combine.c @@ -5479,6 +5479,21 @@ subst (rtx x, rtx from, rtx to, int in_dest, int in_cond, int unique_copy) return x; } +/* If X is a commutative operation whose operands are not in the canonical + order, use substitutions to swap them. */ + +static void +maybe_swap_commutative_operands (rtx x) +{ + if (COMMUTATIVE_ARITH_P (x) + && swap_commutative_operands_p (XEXP (x, 0), XEXP (x, 1))) + { + rtx temp = XEXP (x, 0); + SUBST (XEXP (x, 0), XEXP (x, 1)); + SUBST (XEXP (x, 1), temp); + } +} + /* Simplify X, a piece of RTL. We just operate on the expression at the outer level; call `subst' to simplify recursively. Return the new expression. @@ -5498,13 +5513,7 @@ combine_simplify_rtx (rtx x, machine_mode op0_mode, int in_dest, /* If this is a commutative operation, put a constant last and a complex expression first. We don't need to do this for comparisons here. */ - if (COMMUTATIVE_ARITH_P (x) - && swap_commutative_operands_p (XEXP (x, 0), XEXP (x, 1))) - { - temp = XEXP (x, 0); - SUBST (XEXP (x, 0), XEXP (x, 1)); - SUBST (XEXP (x, 1), temp); - } + maybe_swap_commutative_operands (x); /* Try to fold this expression in case we have constants that weren't present before. */ @@ -7744,55 +7753,38 @@ extract_left_shift (rtx x, int count) return 0; } -/* Look at the expression rooted at X. Look for expressions - equivalent to ZERO_EXTRACT, SIGN_EXTRACT, ZERO_EXTEND, SIGN_EXTEND. - Form these expressions. - - Return the new rtx, usually just X. +/* Subroutine of make_compound_operation. *X_PTR is the rtx at the current + level of the expression and MODE is its mode. IN_CODE is as for + make_compound_operation. *NEXT_CODE_PTR is the value of IN_CODE + that should be used when recursing on operands of *X_PTR. - Also, for machines like the VAX that don't have logical shift insns, - try to convert logical to arithmetic shift operations in cases where - they are equivalent. This undoes the canonicalizations to logical - shifts done elsewhere. + There are two possible actions: - We try, as much as possible, to re-use rtl expressions to save memory. + - Return null. This tells the caller to recurse on *X_PTR with IN_CODE + equal to *NEXT_CODE_PTR, after which *X_PTR holds the final value. - IN_CODE says what kind of expression we are processing. Normally, it is - SET. In a memory address it is MEM. When processing the arguments of - a comparison or a COMPARE against zero, it is COMPARE, or EQ if more - precisely it is an equality comparison against zero. */ + - Return a new rtx, which the caller returns directly. */ -rtx -make_compound_operation (rtx x, enum rtx_code in_code) +static rtx +make_compound_operation_int (machine_mode mode, rtx *x_ptr, + enum rtx_code in_code, + enum rtx_code *next_code_ptr) { + rtx x = *x_ptr; + enum rtx_code next_code = *next_code_ptr; enum rtx_code code = GET_CODE (x); - machine_mode mode = GET_MODE (x); int mode_width = GET_MODE_PRECISION (mode); rtx rhs, lhs; - enum rtx_code next_code; - int i, j; rtx new_rtx = 0; + int i; rtx tem; - const char *fmt; bool equality_comparison = false; - /* PR rtl-optimization/70944. */ - if (VECTOR_MODE_P (mode)) - return x; - - /* Select the code to be used in recursive calls. Once we are inside an - address, we stay there. If we have a comparison, set to COMPARE, - but once inside, go back to our default of SET. */ - if (in_code == EQ) { equality_comparison = true; in_code = COMPARE; } - next_code = (code == MEM ? MEM - : ((code == COMPARE || COMPARISON_P (x)) - && XEXP (x, 1) == const0_rtx) ? COMPARE - : in_code == COMPARE ? SET : in_code); /* Process depending on the code of this operation. If NEW is set nonzero, it will be returned. */ @@ -7804,8 +7796,7 @@ make_compound_operation (rtx x, enum rtx_code in_code) an address. */ if (in_code == MEM && CONST_INT_P (XEXP (x, 1)) && INTVAL (XEXP (x, 1)) < HOST_BITS_PER_WIDE_INT - && INTVAL (XEXP (x, 1)) >= 0 - && SCALAR_INT_MODE_P (mode)) + && INTVAL (XEXP (x, 1)) >= 0) { HOST_WIDE_INT count = INTVAL (XEXP (x, 1)); HOST_WIDE_INT multval = HOST_WIDE_INT_1 << count; @@ -7826,8 +7817,7 @@ make_compound_operation (rtx x, enum rtx_code in_code) rhs = XEXP (x, 1); lhs = make_compound_operation (lhs, next_code); rhs = make_compound_operation (rhs, next_code); - if (GET_CODE (lhs) == MULT && GET_CODE (XEXP (lhs, 0)) == NEG - && SCALAR_INT_MODE_P (mode)) + if (GET_CODE (lhs) == MULT && GET_CODE (XEXP (lhs, 0)) == NEG) { tem = simplify_gen_binary (MULT, mode, XEXP (XEXP (lhs, 0), 0), XEXP (lhs, 1)); @@ -7846,22 +7836,20 @@ make_compound_operation (rtx x, enum rtx_code in_code) { SUBST (XEXP (x, 0), lhs); SUBST (XEXP (x, 1), rhs); - goto maybe_swap; } - x = gen_lowpart (mode, new_rtx); - goto maybe_swap; + maybe_swap_commutative_operands (x); + return x; case MINUS: lhs = XEXP (x, 0); rhs = XEXP (x, 1); lhs = make_compound_operation (lhs, next_code); rhs = make_compound_operation (rhs, next_code); - if (GET_CODE (rhs) == MULT && GET_CODE (XEXP (rhs, 0)) == NEG - && SCALAR_INT_MODE_P (mode)) + if (GET_CODE (rhs) == MULT && GET_CODE (XEXP (rhs, 0)) == NEG) { tem = simplify_gen_binary (MULT, mode, XEXP (XEXP (rhs, 0), 0), XEXP (rhs, 1)); - new_rtx = simplify_gen_binary (PLUS, mode, tem, lhs); + return simplify_gen_binary (PLUS, mode, tem, lhs); } else if (GET_CODE (rhs) == MULT && (CONST_INT_P (XEXP (rhs, 1)) && INTVAL (XEXP (rhs, 1)) < 0)) @@ -7870,7 +7858,7 @@ make_compound_operation (rtx x, enum rtx_code in_code) simplify_gen_unary (NEG, mode, XEXP (rhs, 1), mode)); - new_rtx = simplify_gen_binary (PLUS, mode, tem, lhs); + return simplify_gen_binary (PLUS, mode, tem, lhs); } else { @@ -7878,7 +7866,6 @@ make_compound_operation (rtx x, enum rtx_code in_code) SUBST (XEXP (x, 1), rhs); return x; } - return gen_lowpart (mode, new_rtx); case AND: /* If the second operand is not a constant, we can't do anything @@ -8171,15 +8158,60 @@ make_compound_operation (rtx x, enum rtx_code in_code) } if (new_rtx) + *x_ptr = gen_lowpart (mode, new_rtx); + *next_code_ptr = next_code; + return NULL_RTX; +} + +/* Look at the expression rooted at X. Look for expressions + equivalent to ZERO_EXTRACT, SIGN_EXTRACT, ZERO_EXTEND, SIGN_EXTEND. + Form these expressions. + + Return the new rtx, usually just X. + + Also, for machines like the VAX that don't have logical shift insns, + try to convert logical to arithmetic shift operations in cases where + they are equivalent. This undoes the canonicalizations to logical + shifts done elsewhere. + + We try, as much as possible, to re-use rtl expressions to save memory. + + IN_CODE says what kind of expression we are processing. Normally, it is + SET. In a memory address it is MEM. When processing the arguments of + a comparison or a COMPARE against zero, it is COMPARE, or EQ if more + precisely it is an equality comparison against zero. */ + +rtx +make_compound_operation (rtx x, enum rtx_code in_code) +{ + enum rtx_code code = GET_CODE (x); + const char *fmt; + int i, j; + enum rtx_code next_code; + rtx new_rtx, tem; + + /* Select the code to be used in recursive calls. Once we are inside an + address, we stay there. If we have a comparison, set to COMPARE, + but once inside, go back to our default of SET. */ + + next_code = (code == MEM ? MEM + : ((code == COMPARE || COMPARISON_P (x)) + && XEXP (x, 1) == const0_rtx) ? COMPARE + : in_code == COMPARE || in_code == EQ ? SET : in_code); + + if (SCALAR_INT_MODE_P (GET_MODE (x))) { - x = gen_lowpart (mode, new_rtx); + rtx new_rtx = make_compound_operation_int (GET_MODE (x), &x, + in_code, &next_code); + if (new_rtx) + return new_rtx; code = GET_CODE (x); } /* Now recursively process each operand of this operation. We need to handle ZERO_EXTEND specially so that we don't lose track of the inner mode. */ - if (GET_CODE (x) == ZERO_EXTEND) + if (code == ZERO_EXTEND) { new_rtx = make_compound_operation (XEXP (x, 0), next_code); tem = simplify_const_unary_operation (ZERO_EXTEND, GET_MODE (x), @@ -8204,17 +8236,7 @@ make_compound_operation (rtx x, enum rtx_code in_code) SUBST (XVECEXP (x, i, j), new_rtx); } - maybe_swap: - /* If this is a commutative operation, the changes to the operands - may have made it noncanonical. */ - if (COMMUTATIVE_ARITH_P (x) - && swap_commutative_operands_p (XEXP (x, 0), XEXP (x, 1))) - { - tem = XEXP (x, 0); - SUBST (XEXP (x, 0), XEXP (x, 1)); - SUBST (XEXP (x, 1), tem); - } - + maybe_swap_commutative_operands (x); return x; } @@ -11220,16 +11242,7 @@ change_zero_ext (rtx pat) if (changed) FOR_EACH_SUBRTX_PTR (iter, array, src, NONCONST) - { - rtx x = **iter; - if (COMMUTATIVE_ARITH_P (x) - && swap_commutative_operands_p (XEXP (x, 0), XEXP (x, 1))) - { - rtx tem = XEXP (x, 0); - SUBST (XEXP (x, 0), XEXP (x, 1)); - SUBST (XEXP (x, 1), tem); - } - } + maybe_swap_commutative_operands (**iter); rtx *dst = &SET_DEST (pat); if (GET_CODE (*dst) == ZERO_EXTRACT