Re: [PATCH] riscv: relax splitter restrictions for creating pseudos

[Vineet Gupta <vineetg@rivosinc.com>]

On 4/18/23 11:36, Jeff Law wrote:
>
>
> On 4/18/23 08:36, Vineet Gupta wrote:
>> [partial addressing of PR/109279]
>>
>> RISCV splitters have restrictions to not create pesudos due to a combine
>> limitatation. And despite this being a split-during-combine limitation,
>> all split passes take the hit due to way define*_split are used in gcc.
>>
>> With the original combine issue being fixed 61bee6aed2 ("combine: Don't
>> record for UNDO_MODE pointers into regno_reg_rtx array [PR104985]")
>> the RV splitters can now be relaxed.
>>
>> This improves the codegen in general. e.g.
>>
>>     long long f(void) { return 0x0101010101010101ull; }
>>
>> Before
>>
>>     li    a0,0x01010000
>>     addi    a0,0x0101
>>     slli    a0,a0,16
>>     addi    a0,a0,0x0101
>>     slli    a0,a0,16
>>     addi    a0,a0,0x0101
>>     ret
>>
>> With patch
>>
>>     li    a5,0x01010000
>>     addi    a5,a5,0x0101
>>     mv    a0,a5
>>     slli    a5,a5,32
>>     add    a0,a5,a0
>>     ret
>>
>> This is testsuite clean, no regression w/ patch.
>>
>>                 ========= Summary of gcc testsuite =========
>>                              | # of unexpected case / # of unique 
>> unexpected case
>>                              |          gcc |          g++ | gfortran |
>>   rv64imafdc/  lp64d/ medlow |    2 /     2 |    1 /     1 | 6 /     1 |
>>     rv64imac/   lp64/ medlow |    3 /     3 |    1 /     1 | 43 /     
>> 8 |
>>   rv32imafdc/ ilp32d/ medlow |    1 /     1 |    3 /     2 | 6 /     1 |
>>     rv32imac/  ilp32/ medlow |    1 /     1 |    3 /     2 | 43 /     
>> 8 |
>>
>> This came up as part of IRC chat on PR/109279 and was suggested by
>> Andrew Pinski.
>>
>> Signed-off-by: Vineet Gupta <vineetg@rivosinc.com>
>> ---
>>   gcc/config/riscv/riscv-protos.h |  4 +--
>>   gcc/config/riscv/riscv.cc       | 46 +++++++++++++--------------------
>>   gcc/config/riscv/riscv.md       |  8 +++---
>>   3 files changed, 24 insertions(+), 34 deletions(-)
> This looks fine, except that you don't have a ChangeLog.

Oops sorry, yeah realized that right after pressing send. I did post a 
v2 with the changelog.

> It also looks like you don't have write permissions in the repository 
> (not listed in the MAINTAINERS file).  We might as well fix the 
> latter. You can then add a ChangeLog and push this yourself.

That would be awesome. Thx.
>
> Start with this form:
>
> https://sourceware.org/cgi-bin/pdw/ps_form.cgi
>
> Go ahead and list me as approving your request.

Done.

Thx again.
-Vineet