Re: Re: [PATCH 2/2] [RISC-V] Enalble zcmp for -Os

2023-09-11 Thread Fei Gao 
On 2023-09-06 16:06  Kito Cheng  wrote:
>
>On Wed, Sep 6, 2023 at 9:47 AM Fei Gao  wrote:
>>
>> On 2023-09-05 20:02  Kito Cheng  wrote:
>> >
>> >> @@ -5569,7 +5571,9 @@ riscv_avoid_multi_push (const struct 
>> >> riscv_frame_info *frame)
>> >>  {
>> >>    if (!TARGET_ZCMP || crtl->calls_eh_return || frame_pointer_needed
>> >>    || cfun->machine->interrupt_handler_p || 
>> >>cfun->machine->varargs_size != 0
>> >> -  || crtl->args.pretend_args_size != 0 || flag_shrink_wrap_separate
>> >> +  || crtl->args.pretend_args_size != 0
>> >> +  || (use_shrink_wrapping_separate ()
>> >> + && !riscv_avoid_shrink_wrapping_separate ())
>> >
>> >I think we should also check "!optimize_function_for_size_p (cfun)"
>> >here, otherwise that does not really match what we claim in the commit
>> >message.
>> >
>> A similar check optimize_function_for_speed_p is included in
>> use_shrink_wrapping_separate of [1/2] allow targets to check
>> shrink-wrap-separate enabled or not.
>>
>> >e.g. it still will enable with -O2 -fno-shrink-wrap-separate
>> It's intentional to enable zcmp with -O2 -fno-shrink-wrap-separate.
>> Maybe I should have given a better commit message saying
>> "enable muti push and pop for Zcmp extension when
>> shrink-wrap-separate is inactive".
>>
>> Would you like a new patch from me or agree with my
>> explanation and modify commit message in your side?
>
>Could you send a new patch with updated commit message. 
hi Kito

New patch with updated commit message:
https://patchwork.sourceware.org/project/gcc/list/?series=24300

BR, 
Fei
>
>
>>
>> BR
>> Fei
>> >
>> >>    || (frame->mask & ~MULTI_PUSH_GPR_MASK))
>> >>  return true;
>> >>
>>

Re: Re: [PATCH 2/2] [RISC-V] Enalble zcmp for -Os

2023-09-06 Thread Kito Cheng via Gcc-patches 
On Wed, Sep 6, 2023 at 9:47 AM Fei Gao  wrote:
>
> On 2023-09-05 20:02  Kito Cheng  wrote:
> >
> >> @@ -5569,7 +5571,9 @@ riscv_avoid_multi_push (const struct 
> >> riscv_frame_info *frame)
> >>  {
> >>if (!TARGET_ZCMP || crtl->calls_eh_return || frame_pointer_needed
> >>|| cfun->machine->interrupt_handler_p || 
> >> cfun->machine->varargs_size != 0
> >> -  || crtl->args.pretend_args_size != 0 || flag_shrink_wrap_separate
> >> +  || crtl->args.pretend_args_size != 0
> >> +  || (use_shrink_wrapping_separate ()
> >> + && !riscv_avoid_shrink_wrapping_separate ())
> >
> >I think we should also check "!optimize_function_for_size_p (cfun)"
> >here, otherwise that does not really match what we claim in the commit
> >message.
> >
> A similar check optimize_function_for_speed_p is included in
> use_shrink_wrapping_separate of [1/2] allow targets to check
> shrink-wrap-separate enabled or not.
>
> >e.g. it still will enable with -O2 -fno-shrink-wrap-separate
> It's intentional to enable zcmp with -O2 -fno-shrink-wrap-separate.
> Maybe I should have given a better commit message saying
> "enable muti push and pop for Zcmp extension when
> shrink-wrap-separate is inactive".
>
> Would you like a new patch from me or agree with my
> explanation and modify commit message in your side?

Could you send a new patch with updated commit message.


>
> BR
> Fei
> >
> >>|| (frame->mask & ~MULTI_PUSH_GPR_MASK))
> >>  return true;
> >>
>

Re: Re: [PATCH 2/2] [RISC-V] Enalble zcmp for -Os

2023-09-05 Thread Fei Gao 
On 2023-09-05 20:02  Kito Cheng  wrote:
>
>> @@ -5569,7 +5571,9 @@ riscv_avoid_multi_push (const struct riscv_frame_info 
>> *frame)
>>  {
>>    if (!TARGET_ZCMP || crtl->calls_eh_return || frame_pointer_needed
>>    || cfun->machine->interrupt_handler_p || cfun->machine->varargs_size 
>>!= 0
>> -  || crtl->args.pretend_args_size != 0 || flag_shrink_wrap_separate
>> +  || crtl->args.pretend_args_size != 0
>> +  || (use_shrink_wrapping_separate ()
>> + && !riscv_avoid_shrink_wrapping_separate ())
>
>I think we should also check "!optimize_function_for_size_p (cfun)"
>here, otherwise that does not really match what we claim in the commit
>message.
> 
A similar check optimize_function_for_speed_p is included in 
use_shrink_wrapping_separate of [1/2] allow targets to check
shrink-wrap-separate enabled or not.

>e.g. it still will enable with -O2 -fno-shrink-wrap-separate 
It's intentional to enable zcmp with -O2 -fno-shrink-wrap-separate. 
Maybe I should have given a better commit message saying
"enable muti push and pop for Zcmp extension when
shrink-wrap-separate is inactive".

Would you like a new patch from me or agree with my
explanation and modify commit message in your side?

BR
Fei
>
>>    || (frame->mask & ~MULTI_PUSH_GPR_MASK))
>>  return true;
>>

Re: [PATCH 2/2] [RISC-V] Enalble zcmp for -Os

2023-09-05 Thread Kito Cheng via Gcc-patches 
> @@ -5569,7 +5571,9 @@ riscv_avoid_multi_push (const struct riscv_frame_info 
> *frame)
>  {
>if (!TARGET_ZCMP || crtl->calls_eh_return || frame_pointer_needed
>|| cfun->machine->interrupt_handler_p || cfun->machine->varargs_size 
> != 0
> -  || crtl->args.pretend_args_size != 0 || flag_shrink_wrap_separate
> +  || crtl->args.pretend_args_size != 0
> +  || (use_shrink_wrapping_separate ()
> + && !riscv_avoid_shrink_wrapping_separate ())

I think we should also check "!optimize_function_for_size_p (cfun)"
here, otherwise that does not really match what we claim in the commit
message.

e.g. it still will enable with -O2 -fno-shrink-wrap-separate

>|| (frame->mask & ~MULTI_PUSH_GPR_MASK))
>  return true;
>