Re: [PATCH, GCC/ARM, 1/2] Add support for ASRL(reg) and LSLL(reg) instructions for Armv8.1-M Mainline

[Mihail Ionescu <mihail.ionescu@foss.arm.com>]

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



Hi Kyrill,

On 12/11/2019 05:50 PM, Kyrill Tkachov wrote:
> Hi Mihail,
> 
> On 11/14/19 1:54 PM, Mihail Ionescu wrote:
>> Hi,
>>
>> This patch adds the new scalar shift instructions for Armv8.1-M
>> Mainline to the arm backend.
>> This patch is adding the following instructions:
>>
>> ASRL (reg)
>> LSLL (reg)
>>
> 
> Sorry for the delay, very busy time for GCC development :(
> 
> 
>>
>> ChangeLog entry are as follow:
>>
>> *** gcc/ChangeLog ***
>>
>>
>> 2019-11-14  Mihail-Calin Ionescu <mihail.ionescu@arm.com>
>> 2019-11-14  Sudakshina Das  <sudi.das@arm.com>
>>
>>         * config/arm/arm.h (TARGET_MVE): New macro for MVE support.
> 
> 
> I don't see this hunk in the patch... There's a lot of v8.1-M-related 
> patches in flight. Is it defined elsewhere?

Thanks for having a look at this.
Yes, I forgot to remove that bit from the ChangeLog and mention that the 
patch depends on the Armv8.1-M MVE CLI -- 
https://gcc.gnu.org/ml/gcc-patches/2019-11/msg00641.htm which introduces 
the required TARGET_* macros needed. I've updated the ChangeLog to 
reflect that:

*** gcc/ChangeLog ***


2019-12-18  Mihail-Calin Ionescu  <mihail.ionescu@arm.com>
2019-12-18  Sudakshina Das  <sudi.das@arm.com>

	* config/arm/arm.md (ashldi3): Generate thumb2_lsll for TARGET_HAVE_MVE.
	(ashrdi3): Generate thumb2_asrl for TARGET_HAVE_MVE.
	* config/arm/arm.c (arm_hard_regno_mode_ok): Allocate even odd
	register pairs for doubleword quantities for ARMv8.1M-Mainline.
	* config/arm/thumb2.md (thumb2_asrl): New.
	(thumb2_lsll): Likewise.

> 
> 
>>         * config/arm/arm.md (ashldi3): Generate thumb2_lsll for 
>> TARGET_MVE.
>>         (ashrdi3): Generate thumb2_asrl for TARGET_MVE.
>>         * config/arm/arm.c (arm_hard_regno_mode_ok): Allocate even odd
>>         register pairs for doubleword quantities for ARMv8.1M-Mainline.
>>         * config/arm/thumb2.md (thumb2_asrl): New.
>>         (thumb2_lsll): Likewise.
>>
>> *** gcc/testsuite/ChangeLog ***
>>
>> 2019-11-14  Mihail-Calin Ionescu <mihail.ionescu@arm.com>
>> 2019-11-14  Sudakshina Das  <sudi.das@arm.com>
>>
>>         * gcc.target/arm/armv8_1m-shift-reg_1.c: New test.
>>
>> Testsuite shows no regression when run for arm-none-eabi targets.
>>
>> Is this ok for trunk?
>>
>> Thanks
>> Mihail
>>
>>
>> ###############     Attachment also inlined for ease of reply 
>> ###############
>>
>>
>> diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c
>> index 
>> be51df7d14738bc1addeab8ac5a3806778106bce..bf788087a30343269b30cf7054ec29212ad9c572 
>> 100644
>> --- a/gcc/config/arm/arm.c
>> +++ b/gcc/config/arm/arm.c
>> @@ -24454,14 +24454,15 @@ arm_hard_regno_mode_ok (unsigned int regno, 
>> machine_mode mode)
>>
>>    /* We allow almost any value to be stored in the general registers.
>>       Restrict doubleword quantities to even register pairs in ARM state
>> -     so that we can use ldrd.  Do not allow very large Neon structure
>> -     opaque modes in general registers; they would use too many.  */
>> +     so that we can use ldrd and Armv8.1-M Mainline instructions.
>> +     Do not allow very large Neon structure  opaque modes in general
>> +     registers; they would use too many.  */
> 
> 
> This comment now reads:
> 
> "Restrict doubleword quantities to even register pairs in ARM state
>   so that we can use ldrd and Armv8.1-M Mainline instructions."
> 
> Armv8.1-M Mainline is not ARM mode though, so please clarify this 
> comment further.
> 
> Looks ok to me otherwise (I may even have merged this with the second 
> patch, but I'm not complaining about keeping it simple :) )
> 
> Thanks,
> 
> Kyrill
> 

I've now updated the comment to read:
"Restrict doubleword quantities to even register pairs in ARM state
so that we can use ldrd. The same restriction applies for MVE."


Regards,
Mihail

> 
>>    if (regno <= LAST_ARM_REGNUM)
>>      {
>>        if (ARM_NUM_REGS (mode) > 4)
>>          return false;
>>
>> -      if (TARGET_THUMB2)
>> +      if (TARGET_THUMB2 && !TARGET_HAVE_MVE)
>>          return true;
>>
>>        return !(TARGET_LDRD && GET_MODE_SIZE (mode) > 4 && (regno & 1) 
>> != 0);
>> diff --git a/gcc/config/arm/arm.md b/gcc/config/arm/arm.md
>> index 
>> a91a4b941c3f9d2c3d443f9f4639069ae953fb3b..b735f858a6a5c94d02a6765c1b349cdcb5e77ee3 
>> 100644
>> --- a/gcc/config/arm/arm.md
>> +++ b/gcc/config/arm/arm.md
>> @@ -3503,6 +3503,22 @@
>>                     (match_operand:SI 2 "reg_or_int_operand")))]
>>    "TARGET_32BIT"
>>    "
>> +  if (TARGET_HAVE_MVE)
>> +    {
>> +      if (!reg_or_int_operand (operands[2], SImode))
>> +        operands[2] = force_reg (SImode, operands[2]);
>> +
>> +      /* Armv8.1-M Mainline double shifts are not expanded.  */
>> +      if (REG_P (operands[2]))
>> +       {
>> +         if (!reg_overlap_mentioned_p(operands[0], operands[1]))
>> +           emit_insn (gen_movdi (operands[0], operands[1]));
>> +
>> +         emit_insn (gen_thumb2_lsll (operands[0], operands[2]));
>> +         DONE;
>> +       }
>> +    }
>> +
>>    arm_emit_coreregs_64bit_shift (ASHIFT, operands[0], operands[1],
>>                                   operands[2], gen_reg_rtx (SImode),
>>                                   gen_reg_rtx (SImode));
>> @@ -3530,6 +3546,16 @@
>>                       (match_operand:SI 2 "reg_or_int_operand")))]
>>    "TARGET_32BIT"
>>    "
>> +  /* Armv8.1-M Mainline double shifts are not expanded.  */
>> +  if (TARGET_HAVE_MVE && REG_P (operands[2]))
>> +    {
>> +      if (!reg_overlap_mentioned_p(operands[0], operands[1]))
>> +       emit_insn (gen_movdi (operands[0], operands[1]));
>> +
>> +      emit_insn (gen_thumb2_asrl (operands[0], operands[2]));
>> +      DONE;
>> +    }
>> +
>>    arm_emit_coreregs_64bit_shift (ASHIFTRT, operands[0], operands[1],
>>                                   operands[2], gen_reg_rtx (SImode),
>>                                   gen_reg_rtx (SImode));
>> diff --git a/gcc/config/arm/thumb2.md b/gcc/config/arm/thumb2.md
>> index 
>> c08dab233784bd1cbaae147ece795058d2ef234f..3a716ea954ac55b2081121248b930b7f11520ffa 
>> 100644
>> --- a/gcc/config/arm/thumb2.md
>> +++ b/gcc/config/arm/thumb2.md
>> @@ -1645,3 +1645,19 @@
>>    }
>>    [(set_attr "predicable" "yes")]
>>  )
>> +
>> +(define_insn "thumb2_asrl"
>> +  [(set (match_operand:DI 0 "arm_general_register_operand" "+r")
>> +       (ashiftrt:DI (match_dup 0)
>> +                    (match_operand:SI 1 
>> "arm_general_register_operand" "r")))]
>> +  "TARGET_HAVE_MVE"
>> +  "asrl%?\\t%Q0, %R0, %1"
>> +  [(set_attr "predicable" "yes")])
>> +
>> +(define_insn "thumb2_lsll"
>> +  [(set (match_operand:DI 0 "arm_general_register_operand" "+r")
>> +       (ashift:DI (match_dup 0)
>> +                  (match_operand:SI 1 "arm_general_register_operand" 
>> "r")))]
>> +  "TARGET_HAVE_MVE"
>> +  "lsll%?\\t%Q0, %R0, %1"
>> +  [(set_attr "predicable" "yes")])
>> diff --git a/gcc/testsuite/gcc.target/arm/armv8_1m-shift-reg-1.c 
>> b/gcc/testsuite/gcc.target/arm/armv8_1m-shift-reg-1.c
>> new file mode 100644
>> index 
>> 0000000000000000000000000000000000000000..a97e9d687ef66e9642dd1d735125c8ee941fb151 
>>
>> --- /dev/null
>> +++ b/gcc/testsuite/gcc.target/arm/armv8_1m-shift-reg-1.c
>> @@ -0,0 +1,20 @@
>> +/* { dg-do compile } */
>> +/* { dg-options "-O2 -march=armv8.1-m.main+mve -mfloat-abi=softfp" } */
>> +
>> +long long longval2;
>> +int intval2;
>> +
>> +long long int
>> +asrl_reg ()
>> +{
>> + return (longval2 >> intval2);
>> +}
>> +
>> +long long unsigned int
>> +lsll_reg (long long unsigned longval1, int intval1)
>> +{
>> +  return (longval1 << intval1);
>> +}
>> +
>> +/* { dg-final { scan-assembler "asrl\\tr\[0-9\], r\[0-9\], r\[0-9\]" 
>> } } */
>> +/* { dg-final { scan-assembler "lsll\\tr\[0-9\], r\[0-9\], r\[0-9\]" 
>> } } */
>>

[-- Attachment #2: diff.patch --]
[-- Type: text/x-patch, Size: 4194 bytes --]

diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c
index 3fbd54c59ea1de107235d36ad2474e0cbc8452bb..1ae4a843bb4f9ad196a8a96c0b8b0396fa7cd8a0 100644
--- a/gcc/config/arm/arm.c
+++ b/gcc/config/arm/arm.c
@@ -24828,14 +24828,16 @@ arm_hard_regno_mode_ok (unsigned int regno, machine_mode mode)
 
   /* We allow almost any value to be stored in the general registers.
      Restrict doubleword quantities to even register pairs in ARM state
-     so that we can use ldrd.  Do not allow very large Neon structure
-     opaque modes in general registers; they would use too many.  */
+     so that we can use ldrd. The same restriction applies for MVE
+     in order to support Armv8.1-M Mainline instructions.
+     Do not allow very large Neon structure  opaque modes in general
+     registers; they would use too many.  */
   if (regno <= LAST_ARM_REGNUM)
     {
       if (ARM_NUM_REGS (mode) > 4)
 	return false;
 
-      if (TARGET_THUMB2)
+      if (TARGET_THUMB2 && !TARGET_HAVE_MVE)
 	return true;
 
       return !(TARGET_LDRD && GET_MODE_SIZE (mode) > 4 && (regno & 1) != 0);
diff --git a/gcc/config/arm/arm.md b/gcc/config/arm/arm.md
index d3ac2ff54b62206be3c70461b98c5e122ff3b340..fb813ca38a4ed45661a8dab915400ffa75a18ed7 100644
--- a/gcc/config/arm/arm.md
+++ b/gcc/config/arm/arm.md
@@ -4102,6 +4102,22 @@
                    (match_operand:SI 2 "reg_or_int_operand")))]
   "TARGET_32BIT"
   "
+  if (TARGET_HAVE_MVE)
+    {
+      if (!reg_or_int_operand (operands[2], SImode))
+        operands[2] = force_reg (SImode, operands[2]);
+
+      /* Armv8.1-M Mainline double shifts are not expanded.  */
+      if (REG_P (operands[2]))
+	{
+	  if (!reg_overlap_mentioned_p(operands[0], operands[1]))
+	    emit_insn (gen_movdi (operands[0], operands[1]));
+
+	  emit_insn (gen_thumb2_lsll (operands[0], operands[2]));
+	  DONE;
+	}
+    }
+
   arm_emit_coreregs_64bit_shift (ASHIFT, operands[0], operands[1],
 				 operands[2], gen_reg_rtx (SImode),
 				 gen_reg_rtx (SImode));
@@ -4129,6 +4145,16 @@
                      (match_operand:SI 2 "reg_or_int_operand")))]
   "TARGET_32BIT"
   "
+  /* Armv8.1-M Mainline double shifts are not expanded.  */
+  if (TARGET_HAVE_MVE && REG_P (operands[2]))
+    {
+      if (!reg_overlap_mentioned_p(operands[0], operands[1]))
+	emit_insn (gen_movdi (operands[0], operands[1]));
+
+      emit_insn (gen_thumb2_asrl (operands[0], operands[2]));
+      DONE;
+    }
+
   arm_emit_coreregs_64bit_shift (ASHIFTRT, operands[0], operands[1],
 				 operands[2], gen_reg_rtx (SImode),
 				 gen_reg_rtx (SImode));
diff --git a/gcc/config/arm/thumb2.md b/gcc/config/arm/thumb2.md
index 8c08629a9e5e0621073132dc0e41a788b3581273..18b15fa1272cfc3f09f758f260adbd589bc3c05f 100644
--- a/gcc/config/arm/thumb2.md
+++ b/gcc/config/arm/thumb2.md
@@ -1626,3 +1626,19 @@
   }
   [(set_attr "predicable" "yes")]
 )
+
+(define_insn "thumb2_asrl"
+  [(set (match_operand:DI 0 "arm_general_register_operand" "+r")
+	(ashiftrt:DI (match_dup 0)
+		     (match_operand:SI 1 "arm_general_register_operand" "r")))]
+  "TARGET_HAVE_MVE"
+  "asrl%?\\t%Q0, %R0, %1"
+  [(set_attr "predicable" "yes")])
+
+(define_insn "thumb2_lsll"
+  [(set (match_operand:DI 0 "arm_general_register_operand" "+r")
+	(ashift:DI (match_dup 0)
+		   (match_operand:SI 1 "arm_general_register_operand" "r")))]
+  "TARGET_HAVE_MVE"
+  "lsll%?\\t%Q0, %R0, %1"
+  [(set_attr "predicable" "yes")])
diff --git a/gcc/testsuite/gcc.target/arm/armv8_1m-shift-reg-1.c b/gcc/testsuite/gcc.target/arm/armv8_1m-shift-reg-1.c
new file mode 100644
index 0000000000000000000000000000000000000000..a97e9d687ef66e9642dd1d735125c8ee941fb151
--- /dev/null
+++ b/gcc/testsuite/gcc.target/arm/armv8_1m-shift-reg-1.c
@@ -0,0 +1,20 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -march=armv8.1-m.main+mve -mfloat-abi=softfp" } */
+
+long long longval2;
+int intval2;
+
+long long int
+asrl_reg ()
+{
+ return (longval2 >> intval2);
+}
+
+long long unsigned int
+lsll_reg (long long unsigned longval1, int intval1)
+{
+  return (longval1 << intval1);
+}
+
+/* { dg-final { scan-assembler "asrl\\tr\[0-9\], r\[0-9\], r\[0-9\]" } } */
+/* { dg-final { scan-assembler "lsll\\tr\[0-9\], r\[0-9\], r\[0-9\]" } } */