[PATCH 1/2] allow targets to check shrink-wrap-separate enabled or not
No functional changes but restructure and expose use_shrink_wrapping_separate
to the TARGETs.
gcc/ChangeLog:
* shrink-wrap.cc (try_shrink_wrapping_separate):call
use_shrink_wrapping_separate.
(use_shrink_wrapping_separate): wrap the condition
check in use_shrink_wrapping_separate.
* shrink-wrap.h (use_shrink_wrapping_separate): add to extern
---
gcc/shrink-wrap.cc | 22 +++---
gcc/shrink-wrap.h | 1 +
2 files changed, 16 insertions(+), 7 deletions(-)
diff --git a/gcc/shrink-wrap.cc b/gcc/shrink-wrap.cc
index b8d7b557130..28301f04f89 100644
--- a/gcc/shrink-wrap.cc
+++ b/gcc/shrink-wrap.cc
@@ -1776,16 +1776,13 @@ insert_prologue_epilogue_for_components (sbitmap
components)
commit_edge_insertions ();
}
-/* The main entry point to this subpass. FIRST_BB is where the prologue
- would be normally put. */
-void
-try_shrink_wrapping_separate (basic_block first_bb)
+bool
+use_shrink_wrapping_separate (void)
{
- if (!(SHRINK_WRAPPING_ENABLED
- && flag_shrink_wrap_separate
+ if (!(SHRINK_WRAPPING_ENABLED && flag_shrink_wrap_separate
&& optimize_function_for_speed_p (cfun)
&& targetm.shrink_wrap.get_separate_components))
-return;
+return false;
/* We don't handle "strange" functions. */
if (cfun->calls_alloca
@@ -1794,6 +1791,17 @@ try_shrink_wrapping_separate (basic_block first_bb)
|| crtl->calls_eh_return
|| crtl->has_nonlocal_goto
|| crtl->saves_all_registers)
+return false;
+
+ return true;
+}
+
+/* The main entry point to this subpass. FIRST_BB is where the prologue
+ would be normally put. */
+void
+try_shrink_wrapping_separate (basic_block first_bb)
+{
+ if (!use_shrink_wrapping_separate ())
return;
/* Ask the target what components there are. If it returns NULL, don't
diff --git a/gcc/shrink-wrap.h b/gcc/shrink-wrap.h
index 161647711a3..82386c2b712 100644
--- a/gcc/shrink-wrap.h
+++ b/gcc/shrink-wrap.h
@@ -26,6 +26,7 @@ along with GCC; see the file COPYING3. If not see
extern bool requires_stack_frame_p (rtx_insn *, HARD_REG_SET, HARD_REG_SET);
extern void try_shrink_wrapping (edge *entry_edge, rtx_insn *prologue_seq);
extern void try_shrink_wrapping_separate (basic_block first_bb);
+extern bool use_shrink_wrapping_separate (void);
#define SHRINK_WRAPPING_ENABLED \
(flag_shrink_wrap && targetm.have_simple_return ())
--
2.17.1
Re: Re: [PATCH 1/2] allow targets to check shrink-wrap-separate enabled or not
On 2023-08-29 09:46 Jeff Law wrote:
>
>
>
>On 8/28/23 19:28, Fei Gao wrote:
>> On 2023-08-29 06:54 Jeff Law wrote:
>>>
>>>
>>>
>>> On 8/28/23 01:47, Fei Gao wrote:
no functional changes but allow targets to check shrink-wrap-separate
enabled or not.
gcc/ChangeLog:
* shrink-wrap.cc (try_shrink_wrapping_separate):call
use_shrink_wrapping_separate.
(use_shrink_wrapping_separate): wrap the condition
check in use_shrink_wrapping_separate.
* shrink-wrap.h (use_shrink_wrapping_separate): add to extern
>>> So as I mentioned earlier today in the older thread, can we use
>>> override_options to do this?
>>>
>>> If we look at aarch64_override_options we have this:
>>>
>>> /* The pass to insert speculation tracking runs before
>>> shrink-wrapping and the latter does not know how to update the
>>> tracking status. So disable it in this case. */
>>> if (aarch64_track_speculation)
>>> flag_shrink_wrap = 0;
>>>
>>> We kind of want this instead
>>>
>>> if (flag_shrink_wrap)
>>> {
>>> turn off whatever target bits enable the cm.push/cm.pop insns
>>> }
>>>
>>>
>>> This does imply that we have a distinct target flag to enable/disable
>>> those instructions. But that seems like a good thing to have anyway.
>> I'm afraid we cannot simply resolve the confilict based on
>> flag_shrink_wrap/flag_shrink_wrap_separate only, as they're set true from
>> -O1 onwards,
>> which means zcmp is disabled almostly unless
>> -fno-shrink-warp/-fno-shrink-warp-separate
>> are explictly given.
>Yea, but I would generally expect that if someone is really concerned
>about code size, they're probably using -Os which (hopefully) would not
>have shrink-wrapping enabled.
>
>>
>> So after discussion with Kito, we would like to turn on zcmp for -Os and
>> shrink-warp-separate
>> for the speed perfered optimization. use_shrink_wrapping_separate in this
>> patch provide the
>> chance for this check. No new hook is needed.
>Seems reasonable to me if Kito is OK with it.
Thanks Jeff and Kito for the discussion.
Could you please review the new series at your convenience?
https://patchwork.sourceware.org/project/gcc/list/?series=24065
BR,
Fei
>
>jeff
[PATCH 1/2] allow targets to check shrink-wrap-separate enabled or not
No functional changes but restructure and expose use_shrink_wrapping_separate
to the TARGETs.
gcc/ChangeLog:
* shrink-wrap.cc (try_shrink_wrapping_separate):call
use_shrink_wrapping_separate.
(use_shrink_wrapping_separate): wrap the condition
check in use_shrink_wrapping_separate.
* shrink-wrap.h (use_shrink_wrapping_separate): add to extern
---
gcc/shrink-wrap.cc | 22 +++---
gcc/shrink-wrap.h | 1 +
2 files changed, 16 insertions(+), 7 deletions(-)
diff --git a/gcc/shrink-wrap.cc b/gcc/shrink-wrap.cc
index b8d7b557130..28301f04f89 100644
--- a/gcc/shrink-wrap.cc
+++ b/gcc/shrink-wrap.cc
@@ -1776,16 +1776,13 @@ insert_prologue_epilogue_for_components (sbitmap
components)
commit_edge_insertions ();
}
-/* The main entry point to this subpass. FIRST_BB is where the prologue
- would be normally put. */
-void
-try_shrink_wrapping_separate (basic_block first_bb)
+bool
+use_shrink_wrapping_separate (void)
{
- if (!(SHRINK_WRAPPING_ENABLED
- && flag_shrink_wrap_separate
+ if (!(SHRINK_WRAPPING_ENABLED && flag_shrink_wrap_separate
&& optimize_function_for_speed_p (cfun)
&& targetm.shrink_wrap.get_separate_components))
-return;
+return false;
/* We don't handle "strange" functions. */
if (cfun->calls_alloca
@@ -1794,6 +1791,17 @@ try_shrink_wrapping_separate (basic_block first_bb)
|| crtl->calls_eh_return
|| crtl->has_nonlocal_goto
|| crtl->saves_all_registers)
+return false;
+
+ return true;
+}
+
+/* The main entry point to this subpass. FIRST_BB is where the prologue
+ would be normally put. */
+void
+try_shrink_wrapping_separate (basic_block first_bb)
+{
+ if (!use_shrink_wrapping_separate ())
return;
/* Ask the target what components there are. If it returns NULL, don't
diff --git a/gcc/shrink-wrap.h b/gcc/shrink-wrap.h
index 161647711a3..82386c2b712 100644
--- a/gcc/shrink-wrap.h
+++ b/gcc/shrink-wrap.h
@@ -26,6 +26,7 @@ along with GCC; see the file COPYING3. If not see
extern bool requires_stack_frame_p (rtx_insn *, HARD_REG_SET, HARD_REG_SET);
extern void try_shrink_wrapping (edge *entry_edge, rtx_insn *prologue_seq);
extern void try_shrink_wrapping_separate (basic_block first_bb);
+extern bool use_shrink_wrapping_separate (void);
#define SHRINK_WRAPPING_ENABLED \
(flag_shrink_wrap && targetm.have_simple_return ())
--
2.17.1
Re: [PATCH 1/2] allow targets to check shrink-wrap-separate enabled or not
On 8/28/23 19:28, Fei Gao wrote:
On 2023-08-29 06:54 Jeff Law wrote:
On 8/28/23 01:47, Fei Gao wrote:
no functional changes but allow targets to check shrink-wrap-separate enabled
or not.
gcc/ChangeLog:
* shrink-wrap.cc (try_shrink_wrapping_separate):call
use_shrink_wrapping_separate.
(use_shrink_wrapping_separate): wrap the condition
check in use_shrink_wrapping_separate.
* shrink-wrap.h (use_shrink_wrapping_separate): add to extern
So as I mentioned earlier today in the older thread, can we use
override_options to do this?
If we look at aarch64_override_options we have this:
/* The pass to insert speculation tracking runs before
shrink-wrapping and the latter does not know how to update the
tracking status. So disable it in this case. */
if (aarch64_track_speculation)
flag_shrink_wrap = 0;
We kind of want this instead
if (flag_shrink_wrap)
{
turn off whatever target bits enable the cm.push/cm.pop insns
}
This does imply that we have a distinct target flag to enable/disable
those instructions. But that seems like a good thing to have anyway.
I'm afraid we cannot simply resolve the confilict based on
flag_shrink_wrap/flag_shrink_wrap_separate only, as they're set true from -O1
onwards,
which means zcmp is disabled almostly unless
-fno-shrink-warp/-fno-shrink-warp-separate
are explictly given.
Yea, but I would generally expect that if someone is really concerned
about code size, they're probably using -Os which (hopefully) would not
have shrink-wrapping enabled.
So after discussion with Kito, we would like to turn on zcmp for -Os and
shrink-warp-separate
for the speed perfered optimization. use_shrink_wrapping_separate in this patch
provide the
chance for this check. No new hook is needed.
Seems reasonable to me if Kito is OK with it.
jeff
Re: Re: [PATCH 1/2] allow targets to check shrink-wrap-separate enabled or not
On 2023-08-29 06:54 Jeff Law wrote:
>
>
>
>On 8/28/23 01:47, Fei Gao wrote:
>> no functional changes but allow targets to check shrink-wrap-separate
>> enabled or not.
>>
>> gcc/ChangeLog:
>>
>> * shrink-wrap.cc (try_shrink_wrapping_separate):call
>> use_shrink_wrapping_separate.
>> (use_shrink_wrapping_separate): wrap the condition
>> check in use_shrink_wrapping_separate.
>> * shrink-wrap.h (use_shrink_wrapping_separate): add to extern
>So as I mentioned earlier today in the older thread, can we use
>override_options to do this?
>
>If we look at aarch64_override_options we have this:
>
> /* The pass to insert speculation tracking runs before
> shrink-wrapping and the latter does not know how to update the
> tracking status. So disable it in this case. */
> if (aarch64_track_speculation)
> flag_shrink_wrap = 0;
>
>We kind of want this instead
>
> if (flag_shrink_wrap)
> {
> turn off whatever target bits enable the cm.push/cm.pop insns
> }
>
>
>This does imply that we have a distinct target flag to enable/disable
>those instructions. But that seems like a good thing to have anyway.
I'm afraid we cannot simply resolve the confilict based on
flag_shrink_wrap/flag_shrink_wrap_separate only, as they're set true from -O1
onwards,
which means zcmp is disabled almostly unless
-fno-shrink-warp/-fno-shrink-warp-separate
are explictly given.
So after discussion with Kito, we would like to turn on zcmp for -Os and
shrink-warp-separate
for the speed perfered optimization. use_shrink_wrapping_separate in this patch
provide the
chance for this check. No new hook is needed.
Please let me know what you think.
BR,
Fei
>
>jeff
Re: [PATCH 1/2] allow targets to check shrink-wrap-separate enabled or not
On 8/28/23 01:47, Fei Gao wrote:
no functional changes but allow targets to check shrink-wrap-separate enabled
or not.
gcc/ChangeLog:
* shrink-wrap.cc (try_shrink_wrapping_separate):call
use_shrink_wrapping_separate.
(use_shrink_wrapping_separate): wrap the condition
check in use_shrink_wrapping_separate.
* shrink-wrap.h (use_shrink_wrapping_separate): add to extern
So as I mentioned earlier today in the older thread, can we use
override_options to do this?
If we look at aarch64_override_options we have this:
/* The pass to insert speculation tracking runs before
shrink-wrapping and the latter does not know how to update the
tracking status. So disable it in this case. */
if (aarch64_track_speculation)
flag_shrink_wrap = 0;
We kind of want this instead
if (flag_shrink_wrap)
{
turn off whatever target bits enable the cm.push/cm.pop insns
}
This does imply that we have a distinct target flag to enable/disable
those instructions. But that seems like a good thing to have anyway.
jeff
[PATCH 1/2] allow targets to check shrink-wrap-separate enabled or not
no functional changes but allow targets to check shrink-wrap-separate enabled
or not.
gcc/ChangeLog:
* shrink-wrap.cc (try_shrink_wrapping_separate):call
use_shrink_wrapping_separate.
(use_shrink_wrapping_separate): wrap the condition
check in use_shrink_wrapping_separate.
* shrink-wrap.h (use_shrink_wrapping_separate): add to extern
---
gcc/shrink-wrap.cc | 25 +
gcc/shrink-wrap.h | 1 +
2 files changed, 18 insertions(+), 8 deletions(-)
diff --git a/gcc/shrink-wrap.cc b/gcc/shrink-wrap.cc
index b8d7b557130..d534964321a 100644
--- a/gcc/shrink-wrap.cc
+++ b/gcc/shrink-wrap.cc
@@ -1776,16 +1776,14 @@ insert_prologue_epilogue_for_components (sbitmap
components)
commit_edge_insertions ();
}
-/* The main entry point to this subpass. FIRST_BB is where the prologue
- would be normally put. */
-void
-try_shrink_wrapping_separate (basic_block first_bb)
+bool
+use_shrink_wrapping_separate (void)
{
if (!(SHRINK_WRAPPING_ENABLED
- && flag_shrink_wrap_separate
- && optimize_function_for_speed_p (cfun)
- && targetm.shrink_wrap.get_separate_components))
-return;
+&& flag_shrink_wrap_separate
+&& optimize_function_for_speed_p (cfun)
+&& targetm.shrink_wrap.get_separate_components))
+return false;
/* We don't handle "strange" functions. */
if (cfun->calls_alloca
@@ -1794,6 +1792,17 @@ try_shrink_wrapping_separate (basic_block first_bb)
|| crtl->calls_eh_return
|| crtl->has_nonlocal_goto
|| crtl->saves_all_registers)
+return false;
+
+ return true;
+}
+
+/* The main entry point to this subpass. FIRST_BB is where the prologue
+ would be normally put. */
+void
+try_shrink_wrapping_separate (basic_block first_bb)
+{
+ if (!use_shrink_wrapping_separate ())
return;
/* Ask the target what components there are. If it returns NULL, don't
diff --git a/gcc/shrink-wrap.h b/gcc/shrink-wrap.h
index 161647711a3..82386c2b712 100644
--- a/gcc/shrink-wrap.h
+++ b/gcc/shrink-wrap.h
@@ -26,6 +26,7 @@ along with GCC; see the file COPYING3. If not see
extern bool requires_stack_frame_p (rtx_insn *, HARD_REG_SET, HARD_REG_SET);
extern void try_shrink_wrapping (edge *entry_edge, rtx_insn *prologue_seq);
extern void try_shrink_wrapping_separate (basic_block first_bb);
+extern bool use_shrink_wrapping_separate (void);
#define SHRINK_WRAPPING_ENABLED \
(flag_shrink_wrap && targetm.have_simple_return ())
--
2.17.1
