From: Andrew Stubbs <ams@codesourcery.com>
To: "Joseph S. Myers" <joseph@codesourcery.com>
Cc: Bernd Schmidt <bernds@codesourcery.com>,
Richard Earnshaw <rearnsha@arm.com>,
gcc-patches@gcc.gnu.org
Subject: Re: [patch][simplify-rtx] Fix 16-bit -> 64-bit multiply and accumulate
Date: Wed, 25 May 2011 13:42:00 -0000 [thread overview]
Message-ID: <4DDCF7A7.4000200@codesourcery.com> (raw)
In-Reply-To: <Pine.LNX.4.64.1105241928360.21283@digraph.polyomino.org.uk>
[-- Attachment #1: Type: text/plain, Size: 1690 bytes --]
On 24/05/11 20:35, Joseph S. Myers wrote:
> On Tue, 24 May 2011, Andrew Stubbs wrote:
>
>> 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.)
>
> Are there some missing conditions here? The two aren't equivalent in
> general - (extend:SI (mult:HI a b)) multiplies the HImode values in HImode
> (with modulo arithmetic on overflow) before extending the possibly wrapped
> result to SImode.
Ok, I've now modified my patch to prevent it widening regular multiplies.
It now converts
(extend (mult (extend a) (extend b)))
to
(mult (newextend a) (newextend b))
But I also have it convert
(extend (mult (shift a) (extend b)))
to
(mult (newextend (shift a)) (newextend b))
The latter case is to catch widening multiplies that extract a subreg
using a shift. I don't understand why it doesn't just use subreg in the
first place, but apparently it doesn't, and changing it to do that would
no doubt break many existing machine descriptions.
The latter case also happens to catch cases where an extend is
represented by (ashiftrt (ashift x)), which is nice.
I know that, potentially, not all shifted operands are going to be
widening multiplies, but I *think* this should be safe because other
random shift values are unlikely to match a real widening mult
instruction (and if they do then the code would already be broken). If
somebody knows a reason why this isn't safe then I think I'm going to
need some help figuring out what conditions to use.
OK?
Andrew
[-- Attachment #2: canonical-mul.patch --]
[-- Type: text/x-patch, Size: 4484 bytes --]
2011-05-25 Bernd Schmidt <bernds@codesourcery.com>
Andrew Stubbs <ams@codesourcery.com>
gcc/
* simplify-rtx.c (simplify_unary_operation_1): Canonicalize 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,21 @@ 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} (sign_extend:@var{m2} @var{x})
+(sign_extend:@var{m2} @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_extend}.
+
+@item
+@code{(sign_extend:@var{m1} (mult:@var{m2} (ashiftrt:@var{m2}
+@var{x} @var{s}) (sign_extend:@var{m2} @var{y})))} is converted to
+@code{(mult:@var{m1} (sign_extend:@var{m1} (ashiftrt:@var{m2} @var{x} @var{s}))
+(sign_extend:@var{m1} @var{y}))}, and likewise for patterns using @code{zero_extend}
+and @code{lshiftrt}. If the second operand of @code{mult} is also a shift,
+then that is extended also.
+
@end itemize
Further canonicalization rules are defined in the function
--- a/gcc/simplify-rtx.c
+++ b/gcc/simplify-rtx.c
@@ -1000,6 +1000,34 @@ 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);
+ /* Extending a widening multiplication should be canonicalized to
+ a wider widening multiplication. */
+ if (GET_CODE (op) == MULT)
+ {
+ rtx lhs = XEXP (op, 0);
+ rtx rhs = XEXP (op, 1);
+ enum rtx_code lcode = GET_CODE (lhs);
+ enum rtx_code rcode = GET_CODE (rhs);
+
+ /* Widening multiplies usually extend both operands, but sometimes
+ they use a shift to extract a portion of a register. We assume
+ it is safe to widen all such operands because other examples
+ won't match real instructions. */
+ if ((lcode == SIGN_EXTEND || lcode == ASHIFTRT)
+ && (rcode == SIGN_EXTEND || rcode == ASHIFTRT))
+ {
+ enum machine_mode lmode = GET_MODE (lhs);
+ enum machine_mode rmode = GET_MODE (lhs);
+ return simplify_gen_binary (MULT, mode,
+ simplify_gen_unary (SIGN_EXTEND,
+ mode,
+ lhs, lmode),
+ simplify_gen_unary (SIGN_EXTEND,
+ mode,
+ rhs, rmode));
+ }
+ }
+
/* 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 +1099,34 @@ 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);
+ /* Extending a widening multiplication should be canonicalized to
+ a wider widening multiplication. */
+ if (GET_CODE (op) == MULT)
+ {
+ rtx lhs = XEXP (op, 0);
+ rtx rhs = XEXP (op, 1);
+ enum rtx_code lcode = GET_CODE (lhs);
+ enum rtx_code rcode = GET_CODE (rhs);
+
+ /* Widening multiplies usually extend both operands, but sometimes
+ they use a shift to extract a portion of a register. We assume
+ it is safe to widen all such operands because other examples
+ won't match real instructions. */
+ if ((lcode == ZERO_EXTEND || lcode == LSHIFTRT)
+ && (rcode == ZERO_EXTEND || rcode == LSHIFTRT))
+ {
+ enum machine_mode lmode = GET_MODE (lhs);
+ enum machine_mode rmode = GET_MODE (lhs);
+ return simplify_gen_binary (MULT, mode,
+ simplify_gen_unary (ZERO_EXTEND,
+ mode,
+ lhs, lmode),
+ simplify_gen_unary (ZERO_EXTEND,
+ mode,
+ rhs, rmode));
+ }
+ }
+
/* (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" } } */
next prev parent reply other threads:[~2011-05-25 12:36 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 ` [patch][simplify-rtx] " Andrew Stubbs
2011-05-24 21:00 ` 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 [this message]
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=4DDCF7A7.4000200@codesourcery.com \
--to=ams@codesourcery.com \
--cc=bernds@codesourcery.com \
--cc=gcc-patches@gcc.gnu.org \
--cc=joseph@codesourcery.com \
--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).