* [RFC] Make genautomata.cc output reflect insn-attr.h expectation:
@ 2023-10-31 22:51 Edwin Lu
2023-11-01 15:07 ` Vladimir Makarov
0 siblings, 1 reply; 4+ messages in thread
From: Edwin Lu @ 2023-10-31 22:51 UTC (permalink / raw)
To: gcc-patches; +Cc: gnu-toolchain, jeffreyalaw, Edwin Lu
genattr.cc currently generates insn-attr.h with the following structure:
#if CPU_UNITS_QUERY
extern int get_cpu_unit_code (const char *);
extern int cpu_unit_reservation_p (state_t, int);
#endif
extern bool insn_has_dfa_reservation_p (rtx_insn *);
however genautomata.cc generates insn-automata.cc with the following structure:
#if CPU_UNITS_QUERY
int get_cpu_unit_code (const char * ) { ... }
int cpu_unit_reservation_p (state_t, int) { ... }
bool insn_has_dfa_reservation_p (rtx_insn *) { ... }
#endif
I'm not sure if insn_has_dfa_reservation_p is supposed to be a part of the
CPU_UNITS_QUERY conditional group or not. For consistency, I would like to
move it outside of the group.
This would move insn_has_dfa_reservation_p out of the #if CPU_UNITS_QUERY
conditional inside of insn-automata.cc. This would allow us to see if the
scheduler is trying to schedule an insn with a type which is not associated
with a cpu unit or insn reservation through the TARGET_SCHED_VARIABLE_ISSUE
hook.
If there is a reason for insn_has_dfa_reservation_p being within the
conditional, please let me know!
gcc/Changelog:
* genautomata.cc (write_automata): move endif
Signed-off-by: Edwin Lu <ewlu@rivosinc.com>
---
gcc/genautomata.cc | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/gcc/genautomata.cc b/gcc/genautomata.cc
index 72f01686d6b..9dda25e5ba2 100644
--- a/gcc/genautomata.cc
+++ b/gcc/genautomata.cc
@@ -9503,9 +9503,9 @@ write_automata (void)
fprintf (output_file, "\n#if %s\n\n", CPU_UNITS_QUERY_MACRO_NAME);
output_get_cpu_unit_code_func ();
output_cpu_unit_reservation_p ();
- output_insn_has_dfa_reservation_p ();
fprintf (output_file, "\n#endif /* #if %s */\n\n",
CPU_UNITS_QUERY_MACRO_NAME);
+ output_insn_has_dfa_reservation_p ();
output_dfa_clean_insn_cache_func ();
output_dfa_start_func ();
output_dfa_finish_func ();
--
2.34.1
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [RFC] Make genautomata.cc output reflect insn-attr.h expectation:
2023-10-31 22:51 [RFC] Make genautomata.cc output reflect insn-attr.h expectation: Edwin Lu
@ 2023-11-01 15:07 ` Vladimir Makarov
2023-11-01 17:24 ` [Committed] " Edwin Lu
0 siblings, 1 reply; 4+ messages in thread
From: Vladimir Makarov @ 2023-11-01 15:07 UTC (permalink / raw)
To: Edwin Lu, gcc-patches; +Cc: gnu-toolchain, jeffreyalaw
On 10/31/23 18:51, Edwin Lu wrote:
> genattr.cc currently generates insn-attr.h with the following structure:
>
> #if CPU_UNITS_QUERY
> extern int get_cpu_unit_code (const char *);
> extern int cpu_unit_reservation_p (state_t, int);
> #endif
> extern bool insn_has_dfa_reservation_p (rtx_insn *);
>
> however genautomata.cc generates insn-automata.cc with the following structure:
> #if CPU_UNITS_QUERY
> int get_cpu_unit_code (const char * ) { ... }
> int cpu_unit_reservation_p (state_t, int) { ... }
> bool insn_has_dfa_reservation_p (rtx_insn *) { ... }
> #endif
>
> I'm not sure if insn_has_dfa_reservation_p is supposed to be a part of the
> CPU_UNITS_QUERY conditional group or not. For consistency, I would like to
> move it outside of the group.
No, it should be not considered a part of cpu unit query group. The
function just says that there is any cpu reservation by insns.
Two other functions say that the state is still reserving a particular
cpu unit. Using these 2 functions requires a lot of memory for their
implementation and prevent further dfa minimizations. The functions
should be used mostly for VLIW CPUs when we need this information to
generate machine insns (e.g, ia64 VLIW insn template).
> This would move insn_has_dfa_reservation_p out of the #if CPU_UNITS_QUERY
> conditional inside of insn-automata.cc. This would allow us to see if the
> scheduler is trying to schedule an insn with a type which is not associated
> with a cpu unit or insn reservation through the TARGET_SCHED_VARIABLE_ISSUE
> hook.
>
> If there is a reason for insn_has_dfa_reservation_p being within the
> conditional, please let me know!
It seems a typo.
The patch is ok for me. Thank you for finding this out.
> gcc/Changelog:
>
> * genautomata.cc (write_automata): move endif
>
> Signed-off-by: Edwin Lu <ewlu@rivosinc.com>
> ---
> gcc/genautomata.cc | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/gcc/genautomata.cc b/gcc/genautomata.cc
> index 72f01686d6b..9dda25e5ba2 100644
> --- a/gcc/genautomata.cc
> +++ b/gcc/genautomata.cc
> @@ -9503,9 +9503,9 @@ write_automata (void)
> fprintf (output_file, "\n#if %s\n\n", CPU_UNITS_QUERY_MACRO_NAME);
> output_get_cpu_unit_code_func ();
> output_cpu_unit_reservation_p ();
> - output_insn_has_dfa_reservation_p ();
> fprintf (output_file, "\n#endif /* #if %s */\n\n",
> CPU_UNITS_QUERY_MACRO_NAME);
> + output_insn_has_dfa_reservation_p ();
> output_dfa_clean_insn_cache_func ();
> output_dfa_start_func ();
> output_dfa_finish_func ();
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [Committed] Make genautomata.cc output reflect insn-attr.h expectation:
2023-11-01 15:07 ` Vladimir Makarov
@ 2023-11-01 17:24 ` Edwin Lu
2023-11-01 17:24 ` Edwin Lu
0 siblings, 1 reply; 4+ messages in thread
From: Edwin Lu @ 2023-11-01 17:24 UTC (permalink / raw)
To: gcc-patches; +Cc: gnu-toolchain, jeffreyalaw
On 11/1/2023 8:07 AM, Vladimir Makarov wrote:
>
> On 10/31/23 18:51, Edwin Lu wrote:
>> genattr.cc currently generates insn-attr.h with the following structure:
>>
>> #if CPU_UNITS_QUERY
>> extern int get_cpu_unit_code (const char *);
>> extern int cpu_unit_reservation_p (state_t, int);
>> #endif
>> extern bool insn_has_dfa_reservation_p (rtx_insn *);
>>
>> however genautomata.cc generates insn-automata.cc with the following
>> structure:
>> #if CPU_UNITS_QUERY
>> int get_cpu_unit_code (const char * ) { ... }
>> int cpu_unit_reservation_p (state_t, int) { ... }
>> bool insn_has_dfa_reservation_p (rtx_insn *) { ... }
>> #endif
>>
>> I'm not sure if insn_has_dfa_reservation_p is supposed to be a part of
>> the
>> CPU_UNITS_QUERY conditional group or not. For consistency, I would
>> like to
>> move it outside of the group.
>
> No, it should be not considered a part of cpu unit query group. The
> function just says that there is any cpu reservation by insns.
>
> Two other functions say that the state is still reserving a particular
> cpu unit. Using these 2 functions requires a lot of memory for their
> implementation and prevent further dfa minimizations. The functions
> should be used mostly for VLIW CPUs when we need this information to
> generate machine insns (e.g, ia64 VLIW insn template).
>
Thanks for the clarification!
>> This would move insn_has_dfa_reservation_p out of the #if CPU_UNITS_QUERY
>> conditional inside of insn-automata.cc. This would allow us to see if the
>> scheduler is trying to schedule an insn with a type which is not
>> associated
>> with a cpu unit or insn reservation through the
>> TARGET_SCHED_VARIABLE_ISSUE
>> hook.
>>
>> If there is a reason for insn_has_dfa_reservation_p being within the
>> conditional, please let me know!
>
> It seems a typo.
>
> The patch is ok for me. Thank you for finding this out.
>
Committed!
Edwin
>> gcc/Changelog:
>>
>> * genautomata.cc (write_automata): move endif
>>
>> Signed-off-by: Edwin Lu <ewlu@rivosinc.com>
>> ---
>> gcc/genautomata.cc | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/gcc/genautomata.cc b/gcc/genautomata.cc
>> index 72f01686d6b..9dda25e5ba2 100644
>> --- a/gcc/genautomata.cc
>> +++ b/gcc/genautomata.cc
>> @@ -9503,9 +9503,9 @@ write_automata (void)
>> fprintf (output_file, "\n#if %s\n\n", CPU_UNITS_QUERY_MACRO_NAME);
>> output_get_cpu_unit_code_func ();
>> output_cpu_unit_reservation_p ();
>> - output_insn_has_dfa_reservation_p ();
>> fprintf (output_file, "\n#endif /* #if %s */\n\n",
>> CPU_UNITS_QUERY_MACRO_NAME);
>> + output_insn_has_dfa_reservation_p ();
>> output_dfa_clean_insn_cache_func ();
>> output_dfa_start_func ();
>> output_dfa_finish_func ();
>
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [Committed] Make genautomata.cc output reflect insn-attr.h expectation:
2023-11-01 17:24 ` [Committed] " Edwin Lu
@ 2023-11-01 17:24 ` Edwin Lu
0 siblings, 0 replies; 4+ messages in thread
From: Edwin Lu @ 2023-11-01 17:24 UTC (permalink / raw)
To: Vladimir Makarov, gcc-patches; +Cc: gnu-toolchain, jeffreyalaw
On 11/1/2023 8:07 AM, Vladimir Makarov wrote:
>
> On 10/31/23 18:51, Edwin Lu wrote:
>> genattr.cc currently generates insn-attr.h with the following structure:
>>
>> #if CPU_UNITS_QUERY
>> extern int get_cpu_unit_code (const char *);
>> extern int cpu_unit_reservation_p (state_t, int);
>> #endif
>> extern bool insn_has_dfa_reservation_p (rtx_insn *);
>>
>> however genautomata.cc generates insn-automata.cc with the following
>> structure:
>> #if CPU_UNITS_QUERY
>> int get_cpu_unit_code (const char * ) { ... }
>> int cpu_unit_reservation_p (state_t, int) { ... }
>> bool insn_has_dfa_reservation_p (rtx_insn *) { ... }
>> #endif
>>
>> I'm not sure if insn_has_dfa_reservation_p is supposed to be a part of
>> the
>> CPU_UNITS_QUERY conditional group or not. For consistency, I would
>> like to
>> move it outside of the group.
>
> No, it should be not considered a part of cpu unit query group. The
> function just says that there is any cpu reservation by insns.
>
> Two other functions say that the state is still reserving a particular
> cpu unit. Using these 2 functions requires a lot of memory for their
> implementation and prevent further dfa minimizations. The functions
> should be used mostly for VLIW CPUs when we need this information to
> generate machine insns (e.g, ia64 VLIW insn template).
>
Thanks for the clarification!
>> This would move insn_has_dfa_reservation_p out of the #if CPU_UNITS_QUERY
>> conditional inside of insn-automata.cc. This would allow us to see if the
>> scheduler is trying to schedule an insn with a type which is not
>> associated
>> with a cpu unit or insn reservation through the
>> TARGET_SCHED_VARIABLE_ISSUE
>> hook.
>>
>> If there is a reason for insn_has_dfa_reservation_p being within the
>> conditional, please let me know!
>
> It seems a typo.
>
> The patch is ok for me. Thank you for finding this out.
>
Committed!
Edwin
>> gcc/Changelog:
>>
>> * genautomata.cc (write_automata): move endif
>>
>> Signed-off-by: Edwin Lu <ewlu@rivosinc.com>
>> ---
>> gcc/genautomata.cc | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/gcc/genautomata.cc b/gcc/genautomata.cc
>> index 72f01686d6b..9dda25e5ba2 100644
>> --- a/gcc/genautomata.cc
>> +++ b/gcc/genautomata.cc
>> @@ -9503,9 +9503,9 @@ write_automata (void)
>> fprintf (output_file, "\n#if %s\n\n", CPU_UNITS_QUERY_MACRO_NAME);
>> output_get_cpu_unit_code_func ();
>> output_cpu_unit_reservation_p ();
>> - output_insn_has_dfa_reservation_p ();
>> fprintf (output_file, "\n#endif /* #if %s */\n\n",
>> CPU_UNITS_QUERY_MACRO_NAME);
>> + output_insn_has_dfa_reservation_p ();
>> output_dfa_clean_insn_cache_func ();
>> output_dfa_start_func ();
>> output_dfa_finish_func ();
>
>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2023-11-01 17:24 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-10-31 22:51 [RFC] Make genautomata.cc output reflect insn-attr.h expectation: Edwin Lu
2023-11-01 15:07 ` Vladimir Makarov
2023-11-01 17:24 ` [Committed] " Edwin Lu
2023-11-01 17:24 ` Edwin Lu
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).