[PATCH][AArch64] Simplify ashl<mode>3 expander for SHORT modes

[PATCH][AArch64] Simplify ashl<mode>3 expander for SHORT modes

[Kyrill Tkachov]

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

Hi all,

The ashl<mode>3 expander for QI and HI modes is needlessly obfuscated.
The 2nd operand predicate accepts nonmemory_operand but the expand code
FAILs if it's not a CONST_INT. We can just demand a const_int_operand in
the predicate and remove the extra CONST_INT check.

Looking at git blame, it seems it was written that way as a result of some
other refactoring a few years back for an unrelated change.

Bootstrapped and tested on aarch64-none-linux-gnu.
Ok for trunk?

Thanks,
Kyrill

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

     * config/aarch64/aarch64.md (ashl<mode>3, SHORT modes):
     Use const_int_operand for operand 2 predicate.  Simplify expand code
     as a result.

[-- Attachment #2: aarch64-ashl-short.patch --]
[-- Type: text/x-patch, Size: 1098 bytes --]

diff --git a/gcc/config/aarch64/aarch64.md b/gcc/config/aarch64/aarch64.md
index d7a669e40f9d4ae863c3e48b73f0eebdecea340d..c08e89bc4eb7b51dbb1e5f893238824caeb5f317 100644
--- a/gcc/config/aarch64/aarch64.md
+++ b/gcc/config/aarch64/aarch64.md
@@ -3770,22 +3770,16 @@ (define_expand "<optab><mode>3"
 (define_expand "ashl<mode>3"
   [(set (match_operand:SHORT 0 "register_operand")
 	(ashift:SHORT (match_operand:SHORT 1 "register_operand")
-		      (match_operand:QI 2 "nonmemory_operand")))]
+		      (match_operand:QI 2 "const_int_operand")))]
   ""
   {
-    if (CONST_INT_P (operands[2]))
-      {
-        operands[2] = GEN_INT (INTVAL (operands[2])
-                               & (GET_MODE_BITSIZE (<MODE>mode) - 1));
+    operands[2] = GEN_INT (INTVAL (operands[2]) & GET_MODE_MASK (<MODE>mode));
 
-        if (operands[2] == const0_rtx)
-          {
-	    emit_insn (gen_mov<mode> (operands[0], operands[1]));
-	    DONE;
-          }
+    if (operands[2] == const0_rtx)
+      {
+	emit_insn (gen_mov<mode> (operands[0], operands[1]));
+	DONE;
       }
-    else
-      FAIL;
   }
 )
 

Re: [PATCH][AArch64] Simplify ashl<mode>3 expander for SHORT modes

[Evandro Menezes]

On 04/27/16 09:10, Kyrill Tkachov wrote:
> 2016-04-27 Kyrylo Tkachov  <kyrylo.tkachov@arm.com>
>
>     * config/aarch64/aarch64.md (ashl<mode>3, SHORT modes):
>     Use const_int_operand for operand 2 predicate.  Simplify expand code
>     as a result.

LGTM

-- 
Evandro Menezes

Re: [PATCH][AArch64] Simplify ashl<mode>3 expander for SHORT modes

[Kyrill Tkachov]

Ping.
https://gcc.gnu.org/ml/gcc-patches/2016-04/msg01651.html

Thanks,
Kyrill

On 27/04/16 15:10, Kyrill Tkachov wrote:
> Hi all,
>
> The ashl<mode>3 expander for QI and HI modes is needlessly obfuscated.
> The 2nd operand predicate accepts nonmemory_operand but the expand code
> FAILs if it's not a CONST_INT. We can just demand a const_int_operand in
> the predicate and remove the extra CONST_INT check.
>
> Looking at git blame, it seems it was written that way as a result of some
> other refactoring a few years back for an unrelated change.
>
> Bootstrapped and tested on aarch64-none-linux-gnu.
> Ok for trunk?
>
> Thanks,
> Kyrill
>
> 2016-04-27  Kyrylo Tkachov  <kyrylo.tkachov@arm.com>
>
>     * config/aarch64/aarch64.md (ashl<mode>3, SHORT modes):
>     Use const_int_operand for operand 2 predicate.  Simplify expand code
>     as a result.

Re: [PATCH][AArch64] Simplify ashl<mode>3 expander for SHORT modes

[Kyrill Tkachov]

Ping.

Thanks,
Kyrill

On 11/05/16 10:01, Kyrill Tkachov wrote:
> Ping.
> https://gcc.gnu.org/ml/gcc-patches/2016-04/msg01651.html
>
> Thanks,
> Kyrill
>
> On 27/04/16 15:10, Kyrill Tkachov wrote:
>> Hi all,
>>
>> The ashl<mode>3 expander for QI and HI modes is needlessly obfuscated.
>> The 2nd operand predicate accepts nonmemory_operand but the expand code
>> FAILs if it's not a CONST_INT. We can just demand a const_int_operand in
>> the predicate and remove the extra CONST_INT check.
>>
>> Looking at git blame, it seems it was written that way as a result of some
>> other refactoring a few years back for an unrelated change.
>>
>> Bootstrapped and tested on aarch64-none-linux-gnu.
>> Ok for trunk?
>>
>> Thanks,
>> Kyrill
>>
>> 2016-04-27  Kyrylo Tkachov  <kyrylo.tkachov@arm.com>
>>
>>     * config/aarch64/aarch64.md (ashl<mode>3, SHORT modes):
>>     Use const_int_operand for operand 2 predicate.  Simplify expand code
>>     as a result.
>

Re: [PATCH][AArch64] Simplify ashl<mode>3 expander for SHORT modes

[James Greenhalgh]

On Wed, Apr 27, 2016 at 03:10:47PM +0100, Kyrill Tkachov wrote:
> Hi all,
> 
> The ashl<mode>3 expander for QI and HI modes is needlessly obfuscated.
> The 2nd operand predicate accepts nonmemory_operand but the expand code
> FAILs if it's not a CONST_INT. We can just demand a const_int_operand in
> the predicate and remove the extra CONST_INT check.
> 
> Looking at git blame, it seems it was written that way as a result of some
> other refactoring a few years back for an unrelated change.
> 
> Bootstrapped and tested on aarch64-none-linux-gnu.
> Ok for trunk?

This is OK.

Thanks for the cleanup,
James

> 2016-04-27  Kyrylo Tkachov  <kyrylo.tkachov@arm.com>
> 
>     * config/aarch64/aarch64.md (ashl<mode>3, SHORT modes):
>     Use const_int_operand for operand 2 predicate.  Simplify expand code
>     as a result.