[PATCH][ARM] Restrict {load,store}_multiple expanders to MAX_LD_STM_OPS regs

[PATCH][ARM] Restrict {load,store}_multiple expanders to MAX_LD_STM_OPS regs

[Kyrill Tkachov]

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

Hi all,

The load/store-multiple expanders reject a number of registers outside of [2-14]
but the arm_gen_{load,store}_multiple functions that they called down to have an even
stricter restriction of <= MAX_LDM_STM_OPS that is <= 4. If load_multiple was called with
a number of regs larger than 4 the assert would trigger and we'd ICE.
This patch fixes that possibility by FAILing in expansion if the number of requested
regs is > MAX_LDM_STM_OPS.

The reason we never hit this currently is that the load,store_multiple standard names are
only used in a single place in expr.c to load the argument registers from to a function call.
By a happy coincidence for arm the number of argument registers is 4 so we never exceed that.
The arm backend never calls the load,store_multiple expanders directly but rather the
arm_gen* functions and it makes sure that they're never called with anything
  > MAX_LDM_STM_OPS.

I hit this issue only due to a buggy change I made to expr.c that ended up requesting a
load_multiple of 5 registers which the expander should have rejected but it didn't,
causing an ICE.

If we add more uses of load/store_multiple in the midend this issue will be a loaded gun.

Bootstrapped and tested on arm-none-linux-gnueabihf.

Ok for trunk?

Thanks,
Kyrill

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

     * config/arm/arm.md (load_multiple): Reject operand 2 greater than
     MAX_LDM_STM_OPS.
     (store_multiple): Likewise.

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

commit dbfb3478c8c15ff38156122ed0a827829cf8c095
Author: Kyrylo Tkachov <kyrylo.tkachov@arm.com>
Date:   Wed Mar 18 11:17:48 2015 +0000

    [ARM] Restrict {load,store}_multiple expanders to MAX_LD_STM_OPS regs

diff --git a/gcc/config/arm/arm.md b/gcc/config/arm/arm.md
index abe2b4c..dcc4446 100644
--- a/gcc/config/arm/arm.md
+++ b/gcc/config/arm/arm.md
@@ -6775,7 +6775,7 @@ (define_expand "load_multiple"
 
   /* Support only fixed point registers.  */
   if (!CONST_INT_P (operands[2])
-      || INTVAL (operands[2]) > 14
+      || INTVAL (operands[2]) > MAX_LDM_STM_OPS
       || INTVAL (operands[2]) < 2
       || !MEM_P (operands[1])
       || !REG_P (operands[0])
@@ -6800,7 +6800,7 @@ (define_expand "store_multiple"
 
   /* Support only fixed point registers.  */
   if (!CONST_INT_P (operands[2])
-      || INTVAL (operands[2]) > 14
+      || INTVAL (operands[2]) > MAX_LDM_STM_OPS
       || INTVAL (operands[2]) < 2
       || !REG_P (operands[1])
       || !MEM_P (operands[0])

Re: [PATCH][ARM] Restrict {load,store}_multiple expanders to MAX_LD_STM_OPS regs

[Ramana Radhakrishnan]

On Tue, Apr 14, 2015 at 1:37 PM, Kyrill Tkachov <kyrylo.tkachov@arm.com> wrote:
> Hi all,
>
> The load/store-multiple expanders reject a number of registers outside of
> [2-14]
> but the arm_gen_{load,store}_multiple functions that they called down to
> have an even
> stricter restriction of <= MAX_LDM_STM_OPS that is <= 4. If load_multiple
> was called with
> a number of regs larger than 4 the assert would trigger and we'd ICE.
> This patch fixes that possibility by FAILing in expansion if the number of
> requested
> regs is > MAX_LDM_STM_OPS.
>
> The reason we never hit this currently is that the load,store_multiple
> standard names are
> only used in a single place in expr.c to load the argument registers from to
> a function call.
> By a happy coincidence for arm the number of argument registers is 4 so we
> never exceed that.
> The arm backend never calls the load,store_multiple expanders directly but
> rather the
> arm_gen* functions and it makes sure that they're never called with anything
>  > MAX_LDM_STM_OPS.
>
> I hit this issue only due to a buggy change I made to expr.c that ended up
> requesting a
> load_multiple of 5 registers which the expander should have rejected but it
> didn't,
> causing an ICE.
>
> If we add more uses of load/store_multiple in the midend this issue will be
> a loaded gun.
>
> Bootstrapped and tested on arm-none-linux-gnueabihf.
>
> Ok for trunk?

Whoops ! Good catch.

Ok.

Ramana

>
> Thanks,
> Kyrill
>
> 2015-04-14  Kyrylo Tkachov  <kyrylo.tkachov@arm.com>
>
>     * config/arm/arm.md (load_multiple): Reject operand 2 greater than
>     MAX_LDM_STM_OPS.
>     (store_multiple): Likewise.

Re: [PATCH][ARM] Restrict {load,store}_multiple expanders to MAX_LD_STM_OPS regs

[Kyrill Tkachov]

On 14/04/15 20:10, Ramana Radhakrishnan wrote:
> On Tue, Apr 14, 2015 at 1:37 PM, Kyrill Tkachov <kyrylo.tkachov@arm.com> wrote:
>> Hi all,
>>
>> The load/store-multiple expanders reject a number of registers outside of
>> [2-14]
>> but the arm_gen_{load,store}_multiple functions that they called down to
>> have an even
>> stricter restriction of <= MAX_LDM_STM_OPS that is <= 4. If load_multiple
>> was called with
>> a number of regs larger than 4 the assert would trigger and we'd ICE.
>> This patch fixes that possibility by FAILing in expansion if the number of
>> requested
>> regs is > MAX_LDM_STM_OPS.
>>
>> The reason we never hit this currently is that the load,store_multiple
>> standard names are
>> only used in a single place in expr.c to load the argument registers from to
>> a function call.
>> By a happy coincidence for arm the number of argument registers is 4 so we
>> never exceed that.
>> The arm backend never calls the load,store_multiple expanders directly but
>> rather the
>> arm_gen* functions and it makes sure that they're never called with anything
>>   > MAX_LDM_STM_OPS.
>>
>> I hit this issue only due to a buggy change I made to expr.c that ended up
>> requesting a
>> load_multiple of 5 registers which the expander should have rejected but it
>> didn't,
>> causing an ICE.
>>
>> If we add more uses of load/store_multiple in the midend this issue will be
>> a loaded gun.
>>
>> Bootstrapped and tested on arm-none-linux-gnueabihf.
>>
>> Ok for trunk?
> Whoops ! Good catch.
>
> Ok.

Thanks, I've committed this with r222357.

Kyrill

>
> Ramana
>
>> Thanks,
>> Kyrill
>>
>> 2015-04-14  Kyrylo Tkachov  <kyrylo.tkachov@arm.com>
>>
>>      * config/arm/arm.md (load_multiple): Reject operand 2 greater than
>>      MAX_LDM_STM_OPS.
>>      (store_multiple): Likewise.