From patchwork Fri Jul 22 12:07:24 2011 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andrew Stubbs X-Patchwork-Id: 106271 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 591F9B6F69 for ; Fri, 22 Jul 2011 22:07:48 +1000 (EST) Received: (qmail 6077 invoked by alias); 22 Jul 2011 12:07:45 -0000 Received: (qmail 6067 invoked by uid 22791); 22 Jul 2011 12:07:43 -0000 X-SWARE-Spam-Status: No, hits=-1.7 required=5.0 tests=AWL, BAYES_00, MISSING_HEADERS, RP_MATCHES_RCVD, TW_TM 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; Fri, 22 Jul 2011 12:07:28 +0000 Received: (qmail 17062 invoked from network); 22 Jul 2011 12:07:27 -0000 Received: from unknown (HELO ?192.168.0.104?) (ams@127.0.0.2) by mail.codesourcery.com with ESMTPA; 22 Jul 2011 12:07:27 -0000 Message-ID: <4E2967FC.3090101@codesourcery.com> Date: Fri, 22 Jul 2011 13:07:24 +0100 From: Andrew Stubbs User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:5.0) Gecko/20110627 Thunderbird/5.0 MIME-Version: 1.0 CC: Richard Guenther , gcc-patches@gcc.gnu.org, patches@linaro.org Subject: Re: [PATCH (9/7)] Widening multiplies with constant inputs References: <4E034EF2.3070503@codesourcery.com> <4E282148.9080100@codesourcery.com> <4E2965A5.1080301@codesourcery.com> In-Reply-To: <4E2965A5.1080301@codesourcery.com> 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 ENOPATCH .... On 22/07/11 12:57, Andrew Stubbs wrote: > On 21/07/11 14:22, Richard Guenther wrote: >> On Thu, Jul 21, 2011 at 2:53 PM, Andrew Stubbs >> wrote: >>> This patch is part bug fix, part better optimization. >>> >>> Firstly, my initial patch series introduced a bug that caused an >>> internal >>> compiler error when the input to a multiply was a constant. This was >>> caused >>> by the gimple verification rejecting such things. I'm not totally >>> clear how >>> this ever worked, but I've corrected it by inserting a temporary >>> SSA_NAME >>> between the constant and the multiply. >> >> Huh? Constant operands should be perfectly fine. What was the error >> you got? > > Ok, so it seems that the fold_convert we thought was redundant in patch > 5 (now moved to patch 2) was in fact responsible for making constants > the right type. > > I've rewritten it to use fold_convert to change the constant. > >>> I also discovered that widening multiply-and-accumulate operations >>> were not >>> recognised if any one of the three inputs were a constant. I've >>> corrected >>> this by adjusting the pattern matching. This also required inserting new >>> SSA_NAMEs to make it work. >> >> See above. > > The pattern matching stuff remains the same, but the constant > conversions have been updated. > >>> In order to insert the new SSA_NAME, I've simply reused the existing >>> type >>> conversion code - the only difference is that the conversion may be a >>> no-op, >>> so it just generates a straight forward assignment. >>> >>> OK? >> >> Nope. I suppose you forget to adjust the constants type? Just >> fold-convert it before using it as input to a macc. > > Done. > > OK? > > Andrew > 2011-07-22 Andrew Stubbs gcc/ * tree-ssa-math-opts.c (is_widening_mult_rhs_p): Handle constants beyond conversions. (convert_mult_to_widen): Convert constant inputs to the right type. (convert_plusminus_to_widen): Don't automatically reject inputs that are not an SSA_NAME. Convert constant inputs to the right type. gcc/testsuite/ * gcc.target/arm/wmul-11.c: New file. * gcc.target/arm/wmul-12.c: New file. * gcc.target/arm/wmul-13.c: New file. --- /dev/null +++ b/gcc/testsuite/gcc.target/arm/wmul-11.c @@ -0,0 +1,11 @@ +/* { dg-do compile } */ +/* { dg-options "-O2" } */ +/* { dg-require-effective-target arm_dsp } */ + +long long +foo (int *b) +{ + return 10 * (long long)*b; +} + +/* { dg-final { scan-assembler "smull" } } */ --- /dev/null +++ b/gcc/testsuite/gcc.target/arm/wmul-12.c @@ -0,0 +1,12 @@ +/* { dg-do compile } */ +/* { dg-options "-O2" } */ +/* { dg-require-effective-target arm_dsp } */ + +long long +foo (int *b, int *c) +{ + int tmp = *b * *c; + return 10 + (long long)tmp; +} + +/* { dg-final { scan-assembler "smlal" } } */ --- /dev/null +++ b/gcc/testsuite/gcc.target/arm/wmul-13.c @@ -0,0 +1,11 @@ +/* { dg-do compile } */ +/* { dg-options "-O2" } */ +/* { dg-require-effective-target arm_dsp } */ + +long long +foo (int *a, int *b) +{ + return *a + (long long)*b * 10; +} + +/* { dg-final { scan-assembler "smlal" } } */ --- a/gcc/tree-ssa-math-opts.c +++ b/gcc/tree-ssa-math-opts.c @@ -1997,6 +1997,13 @@ is_widening_mult_rhs_p (tree type, tree rhs, tree *type_out, type1 = TREE_TYPE (rhs1); } + if (TREE_CODE (rhs1) == INTEGER_CST) + { + *new_rhs_out = rhs1; + *type_out = NULL; + return true; + } + if (TREE_CODE (type1) != TREE_CODE (type) || TYPE_PRECISION (type1) * 2 > TYPE_PRECISION (type)) return false; @@ -2170,6 +2177,12 @@ convert_mult_to_widen (gimple stmt, gimple_stmt_iterator *gsi) rhs2 = build_and_insert_cast (gsi, loc, tmp, rhs2); } + /* Handle constants. */ + if (TREE_CODE (rhs1) == INTEGER_CST) + rhs1 = fold_convert (type1, rhs1); + if (TREE_CODE (rhs2) == INTEGER_CST) + rhs2 = fold_convert (type2, rhs2); + gimple_assign_set_rhs1 (stmt, rhs1); gimple_assign_set_rhs2 (stmt, rhs2); gimple_assign_set_rhs_code (stmt, WIDEN_MULT_EXPR); @@ -2221,8 +2234,6 @@ convert_plusminus_to_widen (gimple_stmt_iterator *gsi, gimple stmt, if (is_gimple_assign (rhs1_stmt)) rhs1_code = gimple_assign_rhs_code (rhs1_stmt); } - else - return false; if (TREE_CODE (rhs2) == SSA_NAME) { @@ -2230,8 +2241,6 @@ convert_plusminus_to_widen (gimple_stmt_iterator *gsi, gimple stmt, if (is_gimple_assign (rhs2_stmt)) rhs2_code = gimple_assign_rhs_code (rhs2_stmt); } - else - return false; /* Allow for one conversion statement between the multiply and addition/subtraction statement. If there are more than @@ -2379,6 +2388,12 @@ convert_plusminus_to_widen (gimple_stmt_iterator *gsi, gimple stmt, add_rhs = build_and_insert_cast (gsi, loc, create_tmp_var (type, NULL), add_rhs); + /* Handle constants. */ + if (TREE_CODE (mult_rhs1) == INTEGER_CST) + rhs1 = fold_convert (type1, mult_rhs1); + if (TREE_CODE (mult_rhs2) == INTEGER_CST) + rhs2 = fold_convert (type2, mult_rhs2); + gimple_assign_set_rhs_with_ops_1 (gsi, wmult_code, mult_rhs1, mult_rhs2, add_rhs); update_stmt (gsi_stmt (*gsi));