On Wed, Dec 21, 2016 at 10:40 AM, Kyrill Tkachov <kyrylo.tkac...@foss.arm.com> wrote: > > On 20/12/16 17:30, Richard Biener wrote: >> >> On December 20, 2016 5:01:19 PM GMT+01:00, Kyrill Tkachov >> <kyrylo.tkac...@foss.arm.com> wrote: >>> >>> Hi all, >>> >>> The testcase in this patch generates bogus assembly for arm with -O1 >>> -mfloat-abi=soft: >>> strd r4, [#0, r3] >>> >>> This is due to non-canonical RTL being generated during expansion: >>> (set (mem:DI (plus:SI (const_int 0 [0]) >>> (reg/f:SI 153)) [0 MEM[symbol: a, index: _26, offset: 0B]+0 S8 A64]) >>> (reg:DI 154)) >>> >>> Note the (plus (const_int 0) (reg)). This is being generated in >>> gen_addr_rtx in tree-ssa-address.c >>> where it creates an explicit PLUS rtx through gen_rtx_PLUS, which >>> doesn't try to canonicalise its arguments >>> or simplify. The correct thing to do is to use simplify_gen_binary that >>> will handle all this properly. >> >> But it has to match up the validity check which passes down exactly the >> same RTL(?) Or does this stem from propagation simplifying a MEM after >> IVOPTs? > > > You mean TARGET_LEGITIMATE_ADDRESS_P? Yes, it gets passed on to that, but > the arm implementation of that > doesn't try to handle non-canonical RTL (plus (const0_rtx) (reg) is not > canonical). > Or do you mean some other check?
Ok, now looking at the actual patch and the code in question. For your testcase it happens that symbol == const0_rtx? In this case please amend the if (symbol) check like we do for the base, thus if (symbol && symbol != const0_rtx) Richard. > Thanks, > Kyrill > > >>> I didn't change the other gen_rtx_PLUS calls in this function as their >>> results is later used in XEXP operations >>> that seem to rely on a PLUS expression being explicitly produced, but >>> this particular call doesn't, so it's okay >>> to change it. With this patch the sane assembly is generated: >>> strd r4, [r3] >>> >>> Bootstrapped and tested on arm-none-linux-gnueabihf, x86_64, >>> aarch64-none-linux-gnu. >>> >>> Ok for trunk? >>> >>> Thanks, >>> Kyrill >>> >>> 2016-12-20 Kyrylo Tkachov <kyrylo.tkac...@arm.com> >>> >>> * tree-ssa-address.c (gen_addr_rtx): Use simplify_gen_binary to add >>> *addr to act_elem. >>> >>> 2016-12-20 Kyrylo Tkachov <kyrylo.tkac...@arm.com> >>> >>> * gcc.dg/20161219.c: New test. >> >> >
