Re: Re: [PATCH 2/2] [RISC-V] Enalble zcmp for -Os
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
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
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
> @@ -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;
>
[PATCH 2/2] [RISC-V] Enalble zcmp for -Os
Enalble zcmp for -Os and shrink-warp-separate for
the speed perfered optimization by default.
To force enabling zcmp multi push/pop in speed perfered case,
fno-shrink-wrap-separate has to be explictly given.
gcc/ChangeLog:
* config/riscv/riscv.cc
(riscv_avoid_shrink_wrapping_separate): wrap the condition check in
riscv_avoid_shrink_wrapping_separate.
(riscv_avoid_multi_push):avoid multi push if shrink_wrapping_separate
is active.
(riscv_get_separate_components):call
riscv_avoid_shrink_wrapping_separate
gcc/testsuite/ChangeLog:
* gcc.target/riscv/rv32e_zcmp.c: remove -fno-shrink-wrap-separate
* gcc.target/riscv/rv32i_zcmp.c: likewise
* gcc.target/riscv/zcmp_push_fpr.c: likewise
* gcc.target/riscv/zcmp_stack_alignment.c: likewise
* gcc.target/riscv/zcmp_shrink_wrap_separate.c: New test.
* gcc.target/riscv/zcmp_shrink_wrap_separate2.c: New test.
---
gcc/config/riscv/riscv.cc | 21 -
gcc/testsuite/gcc.target/riscv/rv32e_zcmp.c | 2 +-
gcc/testsuite/gcc.target/riscv/rv32i_zcmp.c | 2 +-
.../gcc.target/riscv/zcmp_push_fpr.c | 2 +-
.../riscv/zcmp_shrink_wrap_separate.c | 93 +++
.../riscv/zcmp_shrink_wrap_separate2.c| 93 +++
.../gcc.target/riscv/zcmp_stack_alignment.c | 2 +-
7 files changed, 207 insertions(+), 8 deletions(-)
create mode 100644 gcc/testsuite/gcc.target/riscv/zcmp_shrink_wrap_separate.c
create mode 100644 gcc/testsuite/gcc.target/riscv/zcmp_shrink_wrap_separate2.c
diff --git a/gcc/config/riscv/riscv.cc b/gcc/config/riscv/riscv.cc
index 78600ba73b6..3f71000c88b 100644
--- a/gcc/config/riscv/riscv.cc
+++ b/gcc/config/riscv/riscv.cc
@@ -64,6 +64,7 @@ along with GCC; see the file COPYING3. If not see
#include "cfghooks.h"
#include "cfgloop.h"
#include "cfgrtl.h"
+#include "shrink-wrap.h"
#include "sel-sched.h"
#include "sched-int.h"
#include "fold-const.h"
@@ -372,6 +373,7 @@ static const struct riscv_tune_param
optimize_size_tune_info = {
false, /* use_divmod_expansion */
};
+static bool riscv_avoid_shrink_wrapping_separate ();
static tree riscv_handle_fndecl_attribute (tree *, tree, tree, int, bool *);
static tree riscv_handle_type_attribute (tree *, tree, tree, int, bool *);
@@ -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 ())
|| (frame->mask & ~MULTI_PUSH_GPR_MASK))
return true;
@@ -6831,6 +6835,17 @@ riscv_epilogue_uses (unsigned int regno)
return false;
}
+static bool
+riscv_avoid_shrink_wrapping_separate ()
+{
+ if (riscv_use_save_libcall (>machine->frame)
+ || cfun->machine->interrupt_handler_p
+ || !cfun->machine->frame.gp_sp_offset.is_constant ())
+return true;
+
+ return false;
+}
+
/* Implement TARGET_SHRINK_WRAP_GET_SEPARATE_COMPONENTS. */
static sbitmap
@@ -6840,9 +6855,7 @@ riscv_get_separate_components (void)
sbitmap components = sbitmap_alloc (FIRST_PSEUDO_REGISTER);
bitmap_clear (components);
- if (riscv_use_save_libcall (>machine->frame)
- || cfun->machine->interrupt_handler_p
- || !cfun->machine->frame.gp_sp_offset.is_constant ())
+ if (riscv_avoid_shrink_wrapping_separate ())
return components;
offset = cfun->machine->frame.gp_sp_offset.to_constant ();
diff --git a/gcc/testsuite/gcc.target/riscv/rv32e_zcmp.c
b/gcc/testsuite/gcc.target/riscv/rv32e_zcmp.c
index 394459c4ed7..50e443573ad 100644
--- a/gcc/testsuite/gcc.target/riscv/rv32e_zcmp.c
+++ b/gcc/testsuite/gcc.target/riscv/rv32e_zcmp.c
@@ -1,5 +1,5 @@
/* { dg-do compile } */
-/* { dg-options " -Os -march=rv32e_zca_zcmp -mabi=ilp32e -mcmodel=medlow
-fno-shrink-wrap-separate" } */
+/* { dg-options " -Os -march=rv32e_zca_zcmp -mabi=ilp32e -mcmodel=medlow" } */
/* { dg-skip-if "" { *-*-* } {"-O0" "-O1" "-O2" "-Og" "-O3" "-Oz" "-flto"} } */
/* { dg-final { check-function-bodies "**" "" } } */
diff --git a/gcc/testsuite/gcc.target/riscv/rv32i_zcmp.c
b/gcc/testsuite/gcc.target/riscv/rv32i_zcmp.c
index f00338a9d17..ea562b7a233 100644
--- a/gcc/testsuite/gcc.target/riscv/rv32i_zcmp.c
+++ b/gcc/testsuite/gcc.target/riscv/rv32i_zcmp.c
@@ -1,5 +1,5 @@
/* { dg-do compile } */
-/* { dg-options " -Os -march=rv32imaf_zca_zcmp -mabi=ilp32f -mcmodel=medlow
-fno-shrink-wrap-separate" }*/
+/* { dg-options " -Os -march=rv32imaf_zca_zcmp -mabi=ilp32f -mcmodel=medlow"
}*/
/* { dg-skip-if "" { *-*-* } {"-O0" "-O1" "-O2" "-Og" "-O3" "-Oz" "-flto"} } */
/* { dg-final { check-function-bodies "**" "" } } */
diff --git
