public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Andrew Stubbs <ams@codesourcery.com>
To: Bernd Schmidt <bernds@codesourcery.com>
Cc: Richard Earnshaw <rearnsha@arm.com>, gcc-patches@gcc.gnu.org
Subject: Re: [patch][simplify-rtx] Fix 16-bit -> 64-bit multiply and accumulate
Date: Tue, 24 May 2011 17:51:00 -0000	[thread overview]
Message-ID: <4DDBE035.8050901@codesourcery.com> (raw)
In-Reply-To: <4DBFC5D8.1090009@codesourcery.com>

[-- Attachment #1: Type: text/plain, Size: 1359 bytes --]

On 03/05/11 10:07, Bernd Schmidt wrote:
> I tried to fix it with the patch below, which unfortunately doesn't work
> since during combine we don't see the SIGN_EXTEND operations inside the
> MULT, but two shift operations instead. Maybe you can complete it from here?

I've tried to make this patch go various ways, and I always find a test 
case that doesn't quite work. I think we're approaching it from the 
wrong angle.

The problem is that widening multiplies are required to be defined as 
(mult (extend ..) (extend ..)), but when the combiner tries to build a 
widening multiply pattern from a regular multiply it naturally ends up 
as (extend (mult .. ..)). The result is that the patch Benrd posted made 
existing widening multiplies wider, but failed to convert regular 
multiplies to widening ones.

I've created this new, simpler patch that converts

   (extend (mult a b))

into

   (mult (extend a) (extend b))

regardless of what 'a' and 'b' might be. (These are then simplified and 
superfluous extends removed, of course.)

I find that this patch fixes all the testcases I have, and permitted me 
to add support for ARM smlalbt/smlaltb/smlaltt also (I'll post that in a 
separate patch).

It does assume that the outer sign_extend/zero_extend indicates the 
inner extend types though, so I'm not sure if there's a problem there?

OK?

Andrew

[-- Attachment #2: canonical-mul.patch --]
[-- Type: text/x-patch, Size: 3143 bytes --]

2011-05-24  Bernd Schmidt  <bernds@codesourcery.com>
	    Andrew Stubbs  <ams@codesourcery.com>

	gcc/
	* simplify-rtx.c (simplify_unary_operation_1): Create a new
	canonical form for widening multiplies.
	* doc/md.texi (Canonicalization of Instructions): Document widening
	multiply canonicalization.

	gcc/testsuite/
	* gcc.target/arm/mla-2.c: New test.

--- a/gcc/doc/md.texi
+++ b/gcc/doc/md.texi
@@ -5840,6 +5840,11 @@ Equality comparisons of a group of bits (usually a single bit) with zero
 will be written using @code{zero_extract} rather than the equivalent
 @code{and} or @code{sign_extract} operations.
 
+@cindex @code{mult}, canonicalization of
+@item
+@code{(sign_extend:@var{m1} (mult:@var{m2} @var{x} @var{y}))} is converted
+to @code{(mult:@var{m1} (sign_extend:@var{m1} @var{x}) (sign_extend:@var{m1} @var{y}))}, and likewise for @code{zero_extract}.
+
 @end itemize
 
 Further canonicalization rules are defined in the function
--- a/gcc/simplify-rtx.c
+++ b/gcc/simplify-rtx.c
@@ -1000,6 +1000,21 @@ simplify_unary_operation_1 (enum rtx_code code, enum machine_mode mode, rtx op)
 	  && GET_CODE (XEXP (XEXP (op, 0), 1)) == LABEL_REF)
 	return XEXP (op, 0);
 
+      /* Convert (sign_extend (mult ..)) to a canonical widening
+	 mulitplication (mult (sign_extend ..) (sign_extend ..)).  */
+      if (GET_CODE (op) == MULT && GET_MODE (op) < mode)
+	{
+	  rtx lhs = XEXP (op, 0);
+	  rtx rhs = XEXP (op, 1);
+	  enum machine_mode lhs_mode = GET_MODE (lhs);
+	  enum machine_mode rhs_mode = GET_MODE (rhs);
+	  return simplify_gen_binary (MULT, mode,
+				      simplify_gen_unary (SIGN_EXTEND, mode,
+							  lhs, lhs_mode),
+				      simplify_gen_unary (SIGN_EXTEND, mode,
+							  rhs, rhs_mode));
+	}
+
       /* Check for a sign extension of a subreg of a promoted
 	 variable, where the promotion is sign-extended, and the
 	 target mode is the same as the variable's promotion.  */
@@ -1071,6 +1086,21 @@ simplify_unary_operation_1 (enum rtx_code code, enum machine_mode mode, rtx op)
 	  && GET_MODE_SIZE (mode) <= GET_MODE_SIZE (GET_MODE (XEXP (op, 0))))
 	return rtl_hooks.gen_lowpart_no_emit (mode, op);
 
+      /* Convert (zero_extend (mult ..)) to a canonical widening
+	 mulitplication (mult (zero_extend ..) (zero_extend ..)).  */
+      if (GET_CODE (op) == MULT && GET_MODE (op) < mode)
+	{
+	  rtx lhs = XEXP (op, 0);
+	  rtx rhs = XEXP (op, 1);
+	  enum machine_mode lhs_mode = GET_MODE (lhs);
+	  enum machine_mode rhs_mode = GET_MODE (rhs);
+	  return simplify_gen_binary (MULT, mode,
+				      simplify_gen_unary (ZERO_EXTEND, mode,
+							  lhs, lhs_mode),
+				      simplify_gen_unary (ZERO_EXTEND, mode,
+							  rhs, rhs_mode));
+	}
+
       /* (zero_extend:M (zero_extend:N <X>)) is (zero_extend:M <X>).  */
       if (GET_CODE (op) == ZERO_EXTEND)
 	return simplify_gen_unary (ZERO_EXTEND, mode, XEXP (op, 0),
--- /dev/null
+++ b/gcc/testsuite/gcc.target/arm/mla-2.c
@@ -0,0 +1,9 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -march=armv7-a" } */
+
+long long foolong (long long x, short *a, short *b)
+{
+    return x + *a * *b;
+}
+
+/* { dg-final { scan-assembler "smlalbb" } } */

  parent reply	other threads:[~2011-05-24 16:43 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-12-09 16:45 [patch][ARM] " Andrew Stubbs
2011-01-27  2:09 ` Ramana Radhakrishnan
2011-01-27 10:46   ` Andrew Stubbs
2011-01-27 19:22 ` Richard Earnshaw
2011-01-28 13:06   ` Andrew Stubbs
2011-01-28 15:08     ` Richard Earnshaw
2011-01-28 15:55       ` Andrew Stubbs
2011-01-28 16:01         ` Richard Earnshaw
2011-03-25 16:27           ` Andrew Stubbs
2011-04-15 11:23             ` Andrew Stubbs
2011-05-03  9:08               ` Bernd Schmidt
2011-05-03  9:36                 ` Andrew Stubbs
2011-05-18 15:32                 ` [patch][combine] " Andrew Stubbs
2011-05-19 13:06                   ` Bernd Schmidt
2011-05-24 17:51                 ` Andrew Stubbs [this message]
2011-05-24 21:00                   ` [patch][simplify-rtx] " Joseph S. Myers
2011-05-25  9:47                     ` Andrew Stubbs
2011-05-25 12:34                       ` Joseph S. Myers
2011-05-25 13:42                     ` Andrew Stubbs
2011-05-25 13:48                       ` Joseph S. Myers
2011-05-25 14:01                         ` Andrew Stubbs
2011-05-25 14:27                           ` Joseph S. Myers
2011-05-26 13:57                             ` Andrew Stubbs
2011-05-26 14:47                               ` Joseph S. Myers
2011-06-02  9:46                               ` Richard Earnshaw
2011-06-07 11:05                                 ` Andrew Stubbs
2011-02-09  6:31       ` [patch][ARM] " Hans-Peter Nilsson

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4DDBE035.8050901@codesourcery.com \
    --to=ams@codesourcery.com \
    --cc=bernds@codesourcery.com \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=rearnsha@arm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).