On 12/18/19 1:23 PM, Mihail Ionescu wrote:
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.ione...@arm.com>
>> 2019-11-14 Sudakshina Das <sudi....@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.ione...@arm.com>
2019-12-18 Sudakshina Das <sudi....@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.ione...@arm.com>
>> 2019-11-14 Sudakshina Das <sudi....@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."
Ok.
Thanks,
Kyril
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\]"
>> } } */
>>
