public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH][AArch64] Add alternative 'extr' pattern, calculate rtx cost properly
@ 2015-04-27 10:01 Kyrill Tkachov
  2015-04-30 15:53 ` Marcus Shawcroft
  0 siblings, 1 reply; 2+ messages in thread
From: Kyrill Tkachov @ 2015-04-27 10:01 UTC (permalink / raw)
  To: GCC Patches; +Cc: Marcus Shawcroft, Richard Earnshaw, James Greenhalgh

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

Hi all,

We currently have a pattern that will recognise a particular combination of shifts and bitwise-or
as an extr instruction. However, the order of the shifts inside the IOR doesn't have
canonicalisation rules (see rev16 pattern for similar stuff).
This means that for code like:

unsigned long
foo (unsigned long a, unsigned long b)
{
   return (a << 16) |  (b >> 48);
}

we will recognise the extr, but for the equivalent:
unsigned long
foo (unsigned long a, unsigned long b)
{
   return (b >> 48) | (a << 16);
}

we won't, and we'll emit three instructions.
This patch adds the pattern for the alternative order of shifts and allows us to generate
for the above the code:
foo:
         extr    x0, x0, x1, 48
         ret

The zero-extended version is added as well and the rtx costs function is updated to handle
all of these cases.

I've seen this pattern trigger in the gcc code itself in expmed.c where it eliminated a sequence
of orrs and shifts into an extr instruction!

Bootstrapped and tested on aarch64-linux.

Ok for trunk?

Thanks,
Kyrill

2015-04-27  Kyrylo Tkachov  <kyrylo.tkachov@arm.com>

     * config/aarch64/aarch64.md (*extr<mode>5_insn_alt): New pattern.
     (*extrsi5_insn_uxtw_alt): Likewise.
     * config/aarch64/aarch64.c (aarch64_extr_rtx_p): New function.
     (aarch64_rtx_costs, IOR case): Use above to properly cost extr
     operations.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: aarch64-extr.patch --]
[-- Type: text/x-patch; name=aarch64-extr.patch, Size: 4309 bytes --]

commit d45e92b3b8c5837328b7b10682565cacfb566e5b
Author: Kyrylo Tkachov <kyrylo.tkachov@arm.com>
Date:   Mon Mar 2 17:26:38 2015 +0000

    [AArch64] Add alternative 'extr' pattern, calculate rtx cost properly

diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index 860a1dd..ef5a1e4 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -5438,6 +5438,51 @@ aarch64_frint_unspec_p (unsigned int u)
     }
 }
 
+/* Return true iff X is an rtx that will match an extr instruction
+   i.e. as described in the *extr<mode>5_insn family of patterns.
+   OP0 and OP1 will be set to the operands of the shifts involved
+   on success and will be NULL_RTX otherwise.  */
+
+static bool
+aarch64_extr_rtx_p (rtx x, rtx *res_op0, rtx *res_op1)
+{
+  rtx op0, op1;
+  machine_mode mode = GET_MODE (x);
+
+  *res_op0 = NULL_RTX;
+  *res_op1 = NULL_RTX;
+
+  if (GET_CODE (x) != IOR)
+    return false;
+
+  op0 = XEXP (x, 0);
+  op1 = XEXP (x, 1);
+
+  if ((GET_CODE (op0) == ASHIFT && GET_CODE (op1) == LSHIFTRT)
+      || (GET_CODE (op1) == ASHIFT && GET_CODE (op0) == LSHIFTRT))
+    {
+     /* Canonicalise locally to ashift in op0, lshiftrt in op1.  */
+      if (GET_CODE (op1) == ASHIFT)
+        std::swap (op0, op1);
+
+      if (!CONST_INT_P (XEXP (op0, 1)) || !CONST_INT_P (XEXP (op1, 1)))
+        return false;
+
+      unsigned HOST_WIDE_INT shft_amnt_0 = UINTVAL (XEXP (op0, 1));
+      unsigned HOST_WIDE_INT shft_amnt_1 = UINTVAL (XEXP (op1, 1));
+
+      if (shft_amnt_0 < GET_MODE_BITSIZE (mode)
+          && shft_amnt_0 + shft_amnt_1 == GET_MODE_BITSIZE (mode))
+        {
+          *res_op0 = XEXP (op0, 0);
+          *res_op1 = XEXP (op1, 0);
+          return true;
+        }
+    }
+
+  return false;
+}
+
 /* Calculate the cost of calculating (if_then_else (OP0) (OP1) (OP2)),
    storing it in *COST.  Result is true if the total cost of the operation
    has now been calculated.  */
@@ -5968,6 +6013,16 @@ cost_plus:
 
           return true;
         }
+
+      if (aarch64_extr_rtx_p (x, &op0, &op1))
+        {
+          *cost += rtx_cost (op0, IOR, 0, speed)
+                   + rtx_cost (op1, IOR, 1, speed);
+          if (speed)
+            *cost += extra_cost->alu.shift;
+
+          return true;
+        }
     /* Fall through.  */
     case XOR:
     case AND:
diff --git a/gcc/config/aarch64/aarch64.md b/gcc/config/aarch64/aarch64.md
index 1a7f888..17a8755 100644
--- a/gcc/config/aarch64/aarch64.md
+++ b/gcc/config/aarch64/aarch64.md
@@ -3597,6 +3597,21 @@ (define_insn "*extr<mode>5_insn"
   [(set_attr "type" "shift_imm")]
 )
 
+;; There are no canonicalisation rules for ashift and lshiftrt inside an ior
+;; so we have to match both orderings.
+(define_insn "*extr<mode>5_insn_alt"
+  [(set (match_operand:GPI 0 "register_operand" "=r")
+	(ior:GPI  (lshiftrt:GPI (match_operand:GPI 2 "register_operand" "r")
+			        (match_operand 4 "const_int_operand" "n"))
+		  (ashift:GPI (match_operand:GPI 1 "register_operand" "r")
+			      (match_operand 3 "const_int_operand" "n"))))]
+  "UINTVAL (operands[3]) < GET_MODE_BITSIZE (<MODE>mode)
+   && (UINTVAL (operands[3]) + UINTVAL (operands[4])
+       == GET_MODE_BITSIZE (<MODE>mode))"
+  "extr\\t%<w>0, %<w>1, %<w>2, %4"
+  [(set_attr "type" "shift_imm")]
+)
+
 ;; zero_extend version of the above
 (define_insn "*extrsi5_insn_uxtw"
   [(set (match_operand:DI 0 "register_operand" "=r")
@@ -3611,6 +3626,19 @@ (define_insn "*extrsi5_insn_uxtw"
   [(set_attr "type" "shift_imm")]
 )
 
+(define_insn "*extrsi5_insn_uxtw_alt"
+  [(set (match_operand:DI 0 "register_operand" "=r")
+	(zero_extend:DI
+	 (ior:SI (lshiftrt:SI (match_operand:SI 2 "register_operand" "r")
+			       (match_operand 4 "const_int_operand" "n"))
+		 (ashift:SI (match_operand:SI 1 "register_operand" "r")
+			    (match_operand 3 "const_int_operand" "n")))))]
+  "UINTVAL (operands[3]) < 32 &&
+   (UINTVAL (operands[3]) + UINTVAL (operands[4]) == 32)"
+  "extr\\t%w0, %w1, %w2, %4"
+  [(set_attr "type" "shift_imm")]
+)
+
 (define_insn "*ror<mode>3_insn"
   [(set (match_operand:GPI 0 "register_operand" "=r")
 	(rotate:GPI (match_operand:GPI 1 "register_operand" "r")

^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: [PATCH][AArch64] Add alternative 'extr' pattern, calculate rtx cost properly
  2015-04-27 10:01 [PATCH][AArch64] Add alternative 'extr' pattern, calculate rtx cost properly Kyrill Tkachov
@ 2015-04-30 15:53 ` Marcus Shawcroft
  0 siblings, 0 replies; 2+ messages in thread
From: Marcus Shawcroft @ 2015-04-30 15:53 UTC (permalink / raw)
  To: Kyrill Tkachov
  Cc: GCC Patches, Marcus Shawcroft, Richard Earnshaw, James Greenhalgh

On 27 April 2015 at 11:01, Kyrill Tkachov <kyrylo.tkachov@arm.com> wrote:

> 2015-04-27  Kyrylo Tkachov  <kyrylo.tkachov@arm.com>
>
>     * config/aarch64/aarch64.md (*extr<mode>5_insn_alt): New pattern.
>     (*extrsi5_insn_uxtw_alt): Likewise.
>     * config/aarch64/aarch64.c (aarch64_extr_rtx_p): New function.
>     (aarch64_rtx_costs, IOR case): Use above to properly cost extr
>     operations.

OK /Marcus

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2015-04-30 15:37 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-04-27 10:01 [PATCH][AArch64] Add alternative 'extr' pattern, calculate rtx cost properly Kyrill Tkachov
2015-04-30 15:53 ` Marcus Shawcroft

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).