| Message ID | 552D09F1.3040405@arm.com |
|---|---|
| State | New |
| Headers | show |
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.
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.
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])
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.