Re: Re: [PATCH 4/5] RISC-V: Add Zcmp extension supports.
On 2023-05-12 16:12 Sinan wrote:
>
>Hi Fei,
>Sorry for the late reply, I've been busy with moving these days :(.
>Thanks for working on it. I would prefer removing the extra pass for popretz
>if possible ... I will test your patches ASAP.
>BR,
>Sinan
hi Sinan
I posted V2 based on Kito's comment just now.
https://www.mail-archive.com/gcc-patches@gcc.gnu.org/msg307507.html
For popretz, we can discuss further offline if it's convenient to you.
BR,
Fei
>--
>Sender:Fei Gao
>Sent At:2023 May 6 (Sat.) 16:53
>Recipient:Sinan
>Cc:jiawei ; gcc-patches
>Subject:Re: Re: [PATCH 4/5] RISC-V: Add Zcmp extension supports.
>On 2023-05-05 23:57 Sinan wrote:
>>
>>> hi Jiawei
>>>
>>> Please ignore my previous reply. I accidently sent the email before I
>>> finished it.
>>> Sorry for that!
>>>
>>> I downloaded the series of patches from you and found in some cases
>>> it fails to generate zcmp push and pop insns.
>>>
>>> TC:
>>>
>>> char my_getchar();
>>> int test_s0()
>>> {
>>>
>>> int a = my_getchar();
>>> int b = my_getchar();
>>> return a+b;
>>> }
>>>
>>> cc1 -fno-shrink-wrap-separate -O2 -march=rv32e_zca_zcmp -mabi=ilp32e
>>> -mcmodel=medlow test.c
>>>
>>> -fno-shrink-wrap-separate is used here to avoid the impact from
>>> shrink-wrap-separate that is by default
>>> enabled in O2.
>>>
>>> As i'm also interested in Zc*, i did some changes mainly in prologue and
>>> epilogue pass quite simliar to
>>> what has been done for save and restore except the CFI directives due to
>>> reversed order that zcmp
>>> pushes and pops ra, s regs than what save and restore do.
>>>
>>> I will refine and share the code soon for your review.
>>>
>>> BR
>>> Fei
>>Hi Fei,
>>In the current implementation, cm.push will not increase the original
>>adjustment size of the stack pointer. As cm.push uses a minimum adjustment
>>size of 16, and in your example, the adjustment size of sp is 12, so cm.push
>>will not be generated.
>>you can find the check at riscv_use_push_pop
>>> > + */
>>> > + if (base_size > frame_size)
>>> > + return false;
>>> > +
>>And if this check is removed, then you can get the output that you expect.
>>```
>> cm.push {ra,s0},-16
>> call my_getchar
>> mv s0,a0
>> call my_getchar
>> add a0,s0,a0
>> cm.popret {ra,s0},16
>>```
>>In many scenarios of rv32e, cm.push cannot be generated as a result. Perhaps
>>we can remove this check? I haven't tested if it is ok to remove this check,
>>and CC jiawei to help test it.
>>BR,
>>Sinan
>hi Sinan
>Thanks for your reply.
>I posted my codes at
>https://www.mail-archive.com/gcc-patches@gcc.gnu.org/msg306921.html
>In the cover letter, i did some comparision.
>Could you please review?
>Thanks & BR,
>Fei
>>--
>>Sender:Fei Gao
>>Sent At:2023 Apr. 25 (Tue.) 18:12
>>Recipient:jiawei
>>Cc:gcc-patches
>>Subject:[PATCH 4/5] RISC-V: Add Zcmp extension supports.
>>hi Jiawei
>>Please ignore my previous reply. I accidently sent the email before I
>>finished it.
>>Sorry for that!
>>I downloaded the series of patches from you and found in some cases
>>it fails to generate zcmp push and pop insns.
>>TC:
>>char my_getchar();
>>int test_s0()
>>{
>> int a = my_getchar();
>> int b = my_getchar();
>> return a+b;
>>}
>>cc1 -fno-shrink-wrap-separate -O2 -march=rv32e_zca_zcmp -mabi=ilp32e
>>-mcmodel=medlow test.c
>>-fno-shrink-wrap-separate is used here to avoid the impact from
>>shrink-wrap-separate that is by default
>>enabled in O2.
>>As i'm also interested in Zc*, i did some changes mainly in prologue and
>>epilogue pass quite simliar to
>>what has been done for save and restore except the CFI directives due to
>>reversed order that zcmp
>>pushes and pops ra, s regs than what save and restore do.
>>I will refine and share the code soon for your review.
>>BR
>>Fei
>>On Thu Apr 6 06:21:17 GMT 2023 Jiawei jia...@iscas.ac.cn wrote:
>>>
>>>Add Zcmp extension instructions support. Generate push/pop
>>>with follow steps:
>>>
>>> 1. preprocessing:
>>> 1.1. if there is no push rtx, then just return. e.g.
>>> (note 5 1 22 2 [bb 2] NOTE_INSN_BASIC_BLOCK)
>>> (insn/f 22 5 23 2 (set (reg/f:SI 2 sp)
>>> (plus:SI (reg/f:SI 2 sp)
>>> (const_int -32 [0xffe0])))
>>> (nil))
>>> (note 23 22 2 2 NOTE_INSN_PROLOGUE_END)
>>> 1.2. if push rtx exists, then we compute the number of
>>> pushed s-registers, n_sreg.
>>>
>>> push rtx should be find before NOTE_INSN_PROLOGUE_END tag
>>>
>>> [2 and 3 happend simultaneously]
>>>
>>> 2. find valid move pattern, mv sN, aN, where N < n_sreg,
>>> and aN is not used the move pattern, and sN is not
>>> defined before the move pattern (from prologue to the
>>> position of move pattern).
>>>
>>> 3. analysis use and reach of every instruction from prologue
>>> to the position of move pattern.
>>> if any sN is used, then we mark the corresponding argument list
>>> candidate as invalid.
>>> e.g.
>>> push {ra,s0-s3}, {}, -32
>>> sw
Re: Re: [PATCH 4/5] RISC-V: Add Zcmp extension supports.
Hi Fei,
Sorry for the late reply, I've been busy with moving these days :(.
Thanks for working on it. I would prefer removing the extra pass for popretz if
possible ... I will test your patches ASAP.
BR,
Sinan
--
Sender:Fei Gao
Sent At:2023 May 6 (Sat.) 16:53
Recipient:Sinan
Cc:jiawei ; gcc-patches
Subject:Re: Re: [PATCH 4/5] RISC-V: Add Zcmp extension supports.
On 2023-05-05 23:57 Sinan wrote:
>
>> hi Jiawei
>>
>> Please ignore my previous reply. I accidently sent the email before I
>> finished it.
>> Sorry for that!
>>
>> I downloaded the series of patches from you and found in some cases
>> it fails to generate zcmp push and pop insns.
>>
>> TC:
>>
>> char my_getchar();
>> int test_s0()
>> {
>>
>> int a = my_getchar();
>> int b = my_getchar();
>> return a+b;
>> }
>>
>> cc1 -fno-shrink-wrap-separate -O2 -march=rv32e_zca_zcmp -mabi=ilp32e
>> -mcmodel=medlow test.c
>>
>> -fno-shrink-wrap-separate is used here to avoid the impact from
>> shrink-wrap-separate that is by default
>> enabled in O2.
>>
>> As i'm also interested in Zc*, i did some changes mainly in prologue and
>> epilogue pass quite simliar to
>> what has been done for save and restore except the CFI directives due to
>> reversed order that zcmp
>> pushes and pops ra, s regs than what save and restore do.
>>
>> I will refine and share the code soon for your review.
>>
>> BR
>> Fei
>Hi Fei,
>In the current implementation, cm.push will not increase the original
>adjustment size of the stack pointer. As cm.push uses a minimum adjustment
>size of 16, and in your example, the adjustment size of sp is 12, so cm.push
>will not be generated.
>you can find the check at riscv_use_push_pop
>> > + */
>> > + if (base_size > frame_size)
>> > + return false;
>> > +
>And if this check is removed, then you can get the output that you expect.
>```
> cm.push {ra,s0},-16
> call my_getchar
> mv s0,a0
> call my_getchar
> add a0,s0,a0
> cm.popret {ra,s0},16
>```
>In many scenarios of rv32e, cm.push cannot be generated as a result. Perhaps
>we can remove this check? I haven't tested if it is ok to remove this check,
>and CC jiawei to help test it.
>BR,
>Sinan
hi Sinan
Thanks for your reply.
I posted my codes at
https://www.mail-archive.com/gcc-patches@gcc.gnu.org/msg306921.html
In the cover letter, i did some comparision.
Could you please review?
Thanks & BR,
Fei
>--
>Sender:Fei Gao
>Sent At:2023 Apr. 25 (Tue.) 18:12
>Recipient:jiawei
>Cc:gcc-patches
>Subject:[PATCH 4/5] RISC-V: Add Zcmp extension supports.
>hi Jiawei
>Please ignore my previous reply. I accidently sent the email before I finished
>it.
>Sorry for that!
>I downloaded the series of patches from you and found in some cases
>it fails to generate zcmp push and pop insns.
>TC:
>char my_getchar();
>int test_s0()
>{
> int a = my_getchar();
> int b = my_getchar();
> return a+b;
>}
>cc1 -fno-shrink-wrap-separate -O2 -march=rv32e_zca_zcmp -mabi=ilp32e
>-mcmodel=medlow test.c
>-fno-shrink-wrap-separate is used here to avoid the impact from
>shrink-wrap-separate that is by default
>enabled in O2.
>As i'm also interested in Zc*, i did some changes mainly in prologue and
>epilogue pass quite simliar to
>what has been done for save and restore except the CFI directives due to
>reversed order that zcmp
>pushes and pops ra, s regs than what save and restore do.
>I will refine and share the code soon for your review.
>BR
>Fei
>On Thu Apr 6 06:21:17 GMT 2023 Jiawei jia...@iscas.ac.cn wrote:
>>
>>Add Zcmp extension instructions support. Generate push/pop
>>with follow steps:
>>
>> 1. preprocessing:
>> 1.1. if there is no push rtx, then just return. e.g.
>> (note 5 1 22 2 [bb 2] NOTE_INSN_BASIC_BLOCK)
>> (insn/f 22 5 23 2 (set (reg/f:SI 2 sp)
>> (plus:SI (reg/f:SI 2 sp)
>> (const_int -32 [0xffe0])))
>> (nil))
>> (note 23 22 2 2 NOTE_INSN_PROLOGUE_END)
>> 1.2. if push rtx exists, then we compute the number of
>> pushed s-registers, n_sreg.
>>
>> push rtx should be find before NOTE_INSN_PROLOGUE_END tag
>>
>> [2 and 3 happend simultaneously]
>>
>> 2. find valid move pattern, mv sN, aN, where N < n_sreg,
>> and aN is not used the move pattern, and sN is not
>> defined before the move pattern (from prologue to the
>> position of move pattern).
>>
>> 3. analysis use and reach of every instruction from prologue
>> to the position of move pattern.
>> if any sN is used, then we mark the corresponding argument list
>> candidate as invalid.
>> e.g.
>> push {ra,s0-s3}, {}, -32
>> sw s0,44(sp) # s0 is used, then argument list is invalid
>> mv a0,a5 # a0 is defined, then argument list is invalid
>> ...
>> mv s0,a0
>> mv s1,a1
>> mv s2,a2
>>
>> 4. if there is a valid argument list, then replace the pop
>> push parallel insn, and delete mv pattern.
>> if not, skip.
>>
>>All "zcmpe" means Zcmp with RVE extension.
>>The push/pop instrunction implement is mostly finished by
Re: Re: [PATCH 4/5] RISC-V: Add Zcmp extension supports.
On 2023-05-05 23:57 Sinan wrote:
>
>> hi Jiawei
>>
>> Please ignore my previous reply. I accidently sent the email before I
>> finished it.
>> Sorry for that!
>>
>> I downloaded the series of patches from you and found in some cases
>> it fails to generate zcmp push and pop insns.
>>
>> TC:
>>
>> char my_getchar();
>> int test_s0()
>> {
>>
>> int a = my_getchar();
>> int b = my_getchar();
>> return a+b;
>> }
>>
>> cc1 -fno-shrink-wrap-separate -O2 -march=rv32e_zca_zcmp -mabi=ilp32e
>> -mcmodel=medlow test.c
>>
>> -fno-shrink-wrap-separate is used here to avoid the impact from
>> shrink-wrap-separate that is by default
>> enabled in O2.
>>
>> As i'm also interested in Zc*, i did some changes mainly in prologue and
>> epilogue pass quite simliar to
>> what has been done for save and restore except the CFI directives due to
>> reversed order that zcmp
>> pushes and pops ra, s regs than what save and restore do.
>>
>> I will refine and share the code soon for your review.
>>
>> BR
>> Fei
>Hi Fei,
>In the current implementation, cm.push will not increase the original
>adjustment size of the stack pointer. As cm.push uses a minimum adjustment
>size of 16, and in your example, the adjustment size of sp is 12, so cm.push
>will not be generated.
>you can find the check at riscv_use_push_pop
>> > + */
>> > + if (base_size > frame_size)
>> > + return false;
>> > +
>And if this check is removed, then you can get the output that you expect.
>```
> cm.push {ra,s0},-16
> call my_getchar
> mv s0,a0
> call my_getchar
> add a0,s0,a0
> cm.popret {ra,s0},16
>```
>In many scenarios of rv32e, cm.push cannot be generated as a result. Perhaps
>we can remove this check? I haven't tested if it is ok to remove this check,
>and CC jiawei to help test it.
>BR,
>Sinan
hi Sinan
Thanks for your reply.
I posted my codes at
https://www.mail-archive.com/gcc-patches@gcc.gnu.org/msg306921.html
In the cover letter, i did some comparision.
Could you please review?
Thanks & BR,
Fei
>--
>Sender:Fei Gao
>Sent At:2023 Apr. 25 (Tue.) 18:12
>Recipient:jiawei
>Cc:gcc-patches
>Subject:[PATCH 4/5] RISC-V: Add Zcmp extension supports.
>hi Jiawei
>Please ignore my previous reply. I accidently sent the email before I finished
>it.
>Sorry for that!
>I downloaded the series of patches from you and found in some cases
>it fails to generate zcmp push and pop insns.
>TC:
>char my_getchar();
>int test_s0()
>{
> int a = my_getchar();
> int b = my_getchar();
> return a+b;
>}
>cc1 -fno-shrink-wrap-separate -O2 -march=rv32e_zca_zcmp -mabi=ilp32e
>-mcmodel=medlow test.c
>-fno-shrink-wrap-separate is used here to avoid the impact from
>shrink-wrap-separate that is by default
>enabled in O2.
>As i'm also interested in Zc*, i did some changes mainly in prologue and
>epilogue pass quite simliar to
>what has been done for save and restore except the CFI directives due to
>reversed order that zcmp
>pushes and pops ra, s regs than what save and restore do.
>I will refine and share the code soon for your review.
>BR
>Fei
>On Thu Apr 6 06:21:17 GMT 2023 Jiawei jia...@iscas.ac.cn wrote:
>>
>>Add Zcmp extension instructions support. Generate push/pop
>>with follow steps:
>>
>> 1. preprocessing:
>> 1.1. if there is no push rtx, then just return. e.g.
>> (note 5 1 22 2 [bb 2] NOTE_INSN_BASIC_BLOCK)
>> (insn/f 22 5 23 2 (set (reg/f:SI 2 sp)
>> (plus:SI (reg/f:SI 2 sp)
>> (const_int -32 [0xffe0])))
>> (nil))
>> (note 23 22 2 2 NOTE_INSN_PROLOGUE_END)
>> 1.2. if push rtx exists, then we compute the number of
>> pushed s-registers, n_sreg.
>>
>> push rtx should be find before NOTE_INSN_PROLOGUE_END tag
>>
>> [2 and 3 happend simultaneously]
>>
>> 2. find valid move pattern, mv sN, aN, where N < n_sreg,
>> and aN is not used the move pattern, and sN is not
>> defined before the move pattern (from prologue to the
>> position of move pattern).
>>
>> 3. analysis use and reach of every instruction from prologue
>> to the position of move pattern.
>> if any sN is used, then we mark the corresponding argument list
>> candidate as invalid.
>> e.g.
>> push {ra,s0-s3}, {}, -32
>> sw s0,44(sp) # s0 is used, then argument list is invalid
>> mv a0,a5 # a0 is defined, then argument list is invalid
>> ...
>> mv s0,a0
>> mv s1,a1
>> mv s2,a2
>>
>> 4. if there is a valid argument list, then replace the pop
>> push parallel insn, and delete mv pattern.
>> if not, skip.
>>
>>All "zcmpe" means Zcmp with RVE extension.
>>The push/pop instrunction implement is mostly finished by Sinan Lin.
>>
>>Co-Authored by: Sinan Lin
>>Co-Authored by: Simon Cook
>>Co-Authored by: Shihua Liao
>>
>>gcc/ChangeLog:
>>
>> * config.gcc: New object.
>> * config/riscv/predicates.md (riscv_stack_push_operation):
>> New predicate.
>> (riscv_stack_pop_operation): Ditto.
>> (pop_return_value_constant): Ditto.
>> * config/riscv/riscv-passes.def (INSERT_PASS_AFTER): New pass.
>> * config/riscv/riscv-protos.h
