From patchwork Tue Aug 3 14:46:48 2010 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Bernd Schmidt X-Patchwork-Id: 60756 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]) by ozlabs.org (Postfix) with SMTP id 92CD4B6F11 for ; Wed, 4 Aug 2010 00:47:08 +1000 (EST) Received: (qmail 9122 invoked by alias); 3 Aug 2010 14:47:07 -0000 Received: (qmail 9113 invoked by uid 22791); 3 Aug 2010 14:47:06 -0000 X-SWARE-Spam-Status: No, hits=-1.8 required=5.0 tests=AWL, BAYES_00, T_LOTS_OF_MONEY, T_RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from mail.codesourcery.com (HELO mail.codesourcery.com) (38.113.113.100) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Tue, 03 Aug 2010 14:47:00 +0000 Received: (qmail 23066 invoked from network); 3 Aug 2010 14:46:58 -0000 Received: from unknown (HELO ?84.152.232.205?) (bernds@127.0.0.2) by mail.codesourcery.com with ESMTPA; 3 Aug 2010 14:46:58 -0000 Message-ID: <4C582BD8.3080306@codesourcery.com> Date: Tue, 03 Aug 2010 16:46:48 +0200 From: Bernd Schmidt User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.2.7) Gecko/20100724 Thunderbird/3.1.1 MIME-Version: 1.0 To: Paolo Bonzini CC: GCC Patches , Richard Earnshaw Subject: Re: Combiner fixes References: <4C572CA0.3040802@codesourcery.com> <4C57C416.70504@gnu.org> In-Reply-To: <4C57C416.70504@gnu.org> 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 On 08/03/2010 09:24 AM, Paolo Bonzini wrote: > On 08/02/2010 10:37 PM, Bernd Schmidt wrote: >> + if (GET_CODE (op0) == NEG && CONST_INT_P (trueop1)) >> + return simplify_gen_binary (MULT, mode, XEXP (op0, 0), >> + simplify_gen_unary (NEG, mode, op1, mode)); > > Why not go one step further and try it on all operands: > > if (GET_CODE (op0) == NEG) > { > rtx temp = simplify_unary (NEG, mode, op1, mode); > if (temp) > return simplify_gen_binary (MULT, mode, XEXP (op0, 0), temp); > } > if (GET_CODE (op1) == NEG) > { > rtx temp = simplify_unary (NEG, mode, op0, mode); > if (temp) > return simplify_gen_binary (MULT, mode, temp, XEXP (op1, 0)); > } Done (slight typo in the above, needs simplify_unary_operation), and also implemented the opposite transformation in combine: (minus x (mult y -12345)) becomes (plus (mult y 12345) x) I've now also looked at code generation on i686, where it also seems to help occasionally: - imull $-12, 4(%ecx), %edx - movl $4, %eax - subl %edx, %eax + imull $12, 4(%ecx), %eax + addl $4, %eax ========= - sall $5, %eax - negl %eax - imull $-2, %eax, %eax + sall $6, %eax There's a single counterexample I found, in 20040709-2.c: - imull $-1029531031, %ecx, %ebp - subl $740551042, %ebp + imull $1103515245, %ecx, %ebp + addl $12345, %ebp + imull $1103515245, %ebp, %ebp + addl $12345, %ebp where an intermediate (minus (const) (mult x const)) is not recognized as a valid pattern in combine, which then prevents later transformations. I think it's one of these cases where combine could benefit from looking at 4 insns. Bootstrapped and regression tested on i686-linux. In the ARM tests, with the previous patch I saw an intermittent segfault on one testcase, which wasn't reproducible when running the compiler manually, and has gone away with the new version (tests still running). I think it's unrelated. Bernd * simplify-rtx.c (simplify_binary_operation_1): Try to simplify away NEG as operand of a MULT by merging it with the other operand. * combine.c (make_compound_operation): Use trunc_int_for_mode when generating a MULT with constant. Canonicalize PLUS and MINUS involving MULT. * config/arm/constraints.md (M): Examine only 32 bits of a HOST_WIDE_INT. * config/arm/predicates.md (power_of_two_operand): Likewise. Index: config/arm/constraints.md =================================================================== --- config/arm/constraints.md.orig +++ config/arm/constraints.md @@ -121,7 +121,7 @@ "In Thumb-1 state a constant that is a multiple of 4 in the range 0-1020." (and (match_code "const_int") (match_test "TARGET_32BIT ? ((ival >= 0 && ival <= 32) - || ((ival & (ival - 1)) == 0)) + || (((ival & (ival - 1)) & 0xFFFFFFFF) == 0)) : ival >= 0 && ival <= 1020 && (ival & 3) == 0"))) (define_constraint "N" Index: config/arm/predicates.md =================================================================== --- config/arm/predicates.md.orig +++ config/arm/predicates.md @@ -289,7 +289,7 @@ (define_predicate "power_of_two_operand" (match_code "const_int") { - HOST_WIDE_INT value = INTVAL (op); + unsigned HOST_WIDE_INT value = INTVAL (op) & 0xffffffff; return value != 0 && (value & (value - 1)) == 0; }) Index: simplify-rtx.c =================================================================== --- simplify-rtx.c.orig +++ simplify-rtx.c @@ -2109,6 +2109,19 @@ simplify_binary_operation_1 (enum rtx_co if (trueop1 == constm1_rtx) return simplify_gen_unary (NEG, mode, op0, mode); + if (GET_CODE (op0) == NEG) + { + rtx temp = simplify_unary_operation (NEG, mode, op1, mode); + if (temp) + return simplify_gen_binary (MULT, mode, XEXP (op0, 0), temp); + } + if (GET_CODE (op1) == NEG) + { + rtx temp = simplify_unary_operation (NEG, mode, op0, mode); + if (temp) + return simplify_gen_binary (MULT, mode, temp, XEXP (op1, 0)); + } + /* Maybe simplify x * 0 to 0. The reduction is not valid if x is NaN, since x * 0 is then also NaN. Nor is it valid when the mode has signed zeros, since multiplying a negative Index: combine.c =================================================================== --- combine.c.orig +++ combine.c @@ -7110,13 +7110,79 @@ make_compound_operation (rtx x, enum rtx && INTVAL (XEXP (x, 1)) < HOST_BITS_PER_WIDE_INT && INTVAL (XEXP (x, 1)) >= 0) { + HOST_WIDE_INT count = INTVAL (XEXP (x, 1)); + HOST_WIDE_INT multval = (HOST_WIDE_INT) 1 << count; + new_rtx = make_compound_operation (XEXP (x, 0), next_code); - new_rtx = gen_rtx_MULT (mode, new_rtx, - GEN_INT ((HOST_WIDE_INT) 1 - << INTVAL (XEXP (x, 1)))); + if (GET_CODE (new_rtx) == NEG) + { + new_rtx = XEXP (new_rtx, 0); + multval = -multval; + } + multval = trunc_int_for_mode (multval, mode); + new_rtx = gen_rtx_MULT (mode, new_rtx, GEN_INT (multval)); } break; + case PLUS: + lhs = XEXP (x, 0); + rhs = XEXP (x, 1); + lhs = make_compound_operation (lhs, MEM); + rhs = make_compound_operation (rhs, MEM); + if (GET_CODE (lhs) == MULT && GET_CODE (XEXP (lhs, 0)) == NEG + && SCALAR_INT_MODE_P (mode)) + { + tem = simplify_gen_binary (MULT, mode, XEXP (XEXP (lhs, 0), 0), + XEXP (lhs, 1)); + new_rtx = simplify_gen_binary (MINUS, mode, rhs, tem); + } + else if (GET_CODE (lhs) == MULT + && (CONST_INT_P (XEXP (lhs, 1)) && INTVAL (XEXP (lhs, 1)) < 0)) + { + tem = simplify_gen_binary (MULT, mode, XEXP (lhs, 0), + simplify_gen_unary (NEG, mode, + XEXP (lhs, 1), + mode)); + new_rtx = simplify_gen_binary (MINUS, mode, rhs, tem); + } + else + { + SUBST (XEXP (x, 0), lhs); + SUBST (XEXP (x, 1), rhs); + goto maybe_swap; + } + x = gen_lowpart (mode, new_rtx); + goto maybe_swap; + + case MINUS: + lhs = XEXP (x, 0); + rhs = XEXP (x, 1); + lhs = make_compound_operation (lhs, MEM); + rhs = make_compound_operation (rhs, MEM); + if (GET_CODE (rhs) == MULT && GET_CODE (XEXP (rhs, 0)) == NEG + && SCALAR_INT_MODE_P (mode)) + { + tem = simplify_gen_binary (MULT, mode, XEXP (XEXP (rhs, 0), 0), + XEXP (rhs, 1)); + new_rtx = simplify_gen_binary (PLUS, mode, tem, lhs); + } + else if (GET_CODE (rhs) == MULT + && (CONST_INT_P (XEXP (rhs, 1)) && INTVAL (XEXP (rhs, 1)) < 0)) + { + tem = simplify_gen_binary (MULT, mode, XEXP (rhs, 0), + simplify_gen_unary (NEG, mode, + XEXP (rhs, 1), + mode)); + new_rtx = simplify_gen_binary (PLUS, mode, tem, lhs); + } + else + { + SUBST (XEXP (x, 0), lhs); + 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 with it. */ @@ -7345,6 +7411,7 @@ make_compound_operation (rtx x, enum rtx 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)