Re: [PATCH v2 1/2] RISC-V: Add shorten_memrefs pass

[Jeff Law <law@redhat.com>]

On 10/26/19 1:10 PM, Oleg Endo wrote:
> On Sat, 2019-10-26 at 12:21 -0600, Jeff Law wrote:
>> On 10/25/19 11:39 AM, Craig Blackmore wrote:
>>> This patch aims to allow more load/store instructions to be
>>> compressed by
>>> replacing a load/store of 'base register + large offset' with a new
>>> load/store
>>> of 'new base + small offset'. If the new base gets stored in a
>>> compressed
>>> register, then the new load/store can be compressed. Since there is
>>> an overhead
>>> in creating the new base, this change is only attempted when 'base
>>> register' is
>>> referenced in at least 4 load/stores in a basic block.
>>>
>>> The optimization is implemented in a new RISC-V specific pass
>>> called
>>> shorten_memrefs which is enabled for RVC targets. It has been
>>> developed for the
>>> 32-bit lw/sw instructions but could also be extended to 64-bit
>>> ld/sd in future.
>>>
>>> Tested on bare metal rv32i, rv32iac, rv32im, rv32imac, rv32imafc,
>>> rv64imac,
>>> rv64imafdc via QEMU. No regressions.
>>>
>>> gcc/ChangeLog:
>>>
>>> 	* config.gcc: Add riscv-shorten-memrefs.o to extra_objs for
>>> riscv.
>>> 	* config/riscv/riscv-passes.def: New file.
>>> 	* config/riscv/riscv-protos.h (make_pass_shorten_memrefs):
>>> Declare.
>>> 	* config/riscv/riscv-shorten-memrefs.c: New file.
>>> 	* config/riscv/riscv.c (tree-pass.h): New include.
>>> 	(riscv_compressed_reg_p): New Function
>>> 	(riscv_compressed_lw_offset_p): Likewise.
>>> 	(riscv_compressed_lw_address_p): Likewise.
>>> 	(riscv_shorten_lw_offset): Likewise.
>>> 	(riscv_legitimize_address): Attempt to convert base +
>>> large_offset
>>>         to compressible new_base + small_offset.
>>> 	(riscv_address_cost): Make anticipated compressed load/stores
>>>         cheaper for code size than uncompressed load/stores.
>>> 	(riscv_register_priority): Move compressed register check to
>>>         riscv_compressed_reg_p.
>>> 	* config/riscv/riscv.h (RISCV_MAX_COMPRESSED_LW_OFFSET):
>>> Define.
>>> 	* config/riscv/riscv.opt (mshorten-memefs): New option.
>>> 	* config/riscv/t-riscv (riscv-shorten-memrefs.o): New rule.
>>> 	(PASSES_EXTRA): Add riscv-passes.def.
>>> 	* doc/invoke.texi: Document -mshorten-memrefs.
>>
>> This has traditionally been done via the the legitimize_address hook.
>> Is there some reason that hook is insufficient for this case?
>>
>> The hook, IIRC, is called out explow.c.
>>
> 
> This sounds like some of my addressing mode selection (AMS) attempts on
> SH.  Haven't looked at the patch (sorry), but I'm sure the problem is
> pretty much the same.
> 
> On SH legitimize_address is used to do ... "something" ... to the
> address in order to make the displacement fit.  The issue is,
> legitimize_address doesn't get any context so it can't even try to find
> a local optimal base address or something like that.
True it's not given any context.  I think most ports try to form the
address in such a way that the base+offset' is likely to be a common
subexpression.

THere was a similar hook for reload which I didn't mention as I wasn't
sure it was used i the LRA world.

SH is likely  than most more complex.  Joern tried really hard to get
good code on the SH and as a result the implementation is sometimes
tough to follow.

Jeff