On Fri, 2017-09-22 at 14:11 -0700, Andrew Pinski wrote: > On Fri, Sep 22, 2017 at 11:39 AM, Jim Wilson <jim.wil...@linaro.org> > wrote: > > > > On Fri, Sep 22, 2017 at 10:58 AM, Andrew Pinski <pins...@gmail.com> > > wrote: > > > > > > Two overall comments: > > > * What about splitting register_offset into two different > > > elements, > > > one for non 128bit modes and one for 128bit (and more; OI, etc.) > > > modes > > > so you get better address generation right away for the simd load > > > cases rather than having LRA/reload having to reload the address > > > into > > > a register. > > I'm not sure if changing register_offset cost would make a > > difference, > > since costs are usually used during optimization, not during > > address > > generation. This is something that I didn't think to try > > though. I > > can try taking a look at this. > It does taken into account when fwprop is propagating the addition > into > the MEM (the tree level is always a_1 = POINTER_PLUS_EXPR; > MEM_REF(a_1)). > IV-OPTS will produce much better code if the address_cost is correct. > > It looks like no other pass (combine, etc.) would take that into > account except for postreload CSE but maybe they should.
I tried increasing the cost of register_offset. This got rid of the reg+reg addressing mode in the middle of the main loop for lmbench stream copy, but did not eliminate it after the main loop. The tree optimized dump has _52 = a_15 + _51; _53 = c_17 + _51; _54 = *_52; *_53 = _54; and the RTL expand dump has (insn 64 63 65 10 (set (reg:DF 96 [ _54 ]) (mem:DF (plus:DI (reg/v/f:DI 78 [ a ]) (reg:DI 93 [ _51 ])) [3 *_52+0 S8 A64])) "stream.c":223 -1 (nil)) (insn 65 64 66 10 (set (mem:DF (plus:DI (reg/v/f:DI 79 [ c ]) (reg:DI 93 [ _51 ])) [3 *_53+0 S8 A64]) (reg:DF 96 [ _54 ])) "stream.c":223 -1 (nil)) That may be fixable, but there is a bigger problem here which is that increasing the costs of register_offset affects both loads and stores. On falkor, it is only quad-word stores that are inefficient with a reg+reg address. Quad-word loads with a reg+reg address are faster than the equivalent add/ldr. Disabling reg+reg address for quad-word loads will hurt performance. Since the address cost stuff makes no distinction between loads and stores, I see no way to get the result I need by using address costs. I can only get the result I need by modifying the md file. > > I did try writing a patch to modify predicates to disallow reg > > offset > > for 128bit modes, and that got complicated, as I had to split apart > > a > > number of patterns in the aarch64-simd.md file that accept both VD > > and > > VQ modes. I ended up with a patch 3-4 times as big as the one I > > submitted, without any additional performance improvement, so it > > wasn't worth the trouble. > > > > > > > > * Maybe adding a testcase to the testsuite to show this change. > > Yes, I can add a testcase. > > > > > > > > One extra comment: > > > * should we change the generic tuning to avoid reg+reg for 128bit > > > modes? > > Are there other targets with a similar problem? I only know that > > it > > is a problem for Falkor. It might be a loss for some targets as it > > is > > replacing one instruction with two. > Well that is why I was suggesting the address cost model change. > Because the cost model change actually might provide better code in > the first place and still allow for reasonable generic code to be > produced. The patch I posted only affects Falkor. It doesn't change generic code. I don't know of any reason why we need to change generic code here. The Falkor core has out-of-order execution and multiple function units, so there isn't any noticeable performance gain from trying to fix this earlier. Fixing this with a md file change gives optimal performance for the testcases I've looked at. Since I'm no longer at Linaro, I expect that someone else will take over this patch submission. I will create a bug report to document the issue, to make it easier to track it and hand off to someone else. Jim
