* [PATCH 1/2] Remove unused remove_node_from_expr_list
@ 2022-07-19 15:15 Alexander Monakov
2022-07-19 15:15 ` [PATCH 2/2] Avoid registering __builtin_setjmp_receiver label twice [PR101347] Alexander Monakov
2022-07-20 7:34 ` [PATCH 1/2] Remove unused remove_node_from_expr_list Richard Biener
0 siblings, 2 replies; 6+ messages in thread
From: Alexander Monakov @ 2022-07-19 15:15 UTC (permalink / raw)
To: gcc-patches; +Cc: Alexander Monakov
This function remains unused since remove_node_from_insn_list was cloned
from it.
gcc/ChangeLog:
* rtl.h (remove_node_from_expr_list): Remove declaration.
* rtlanal.cc (remove_node_from_expr_list): Remove (no uses).
---
gcc/rtl.h | 1 -
gcc/rtlanal.cc | 29 -----------------------------
2 files changed, 30 deletions(-)
diff --git a/gcc/rtl.h b/gcc/rtl.h
index 488016bb4..645c009a3 100644
--- a/gcc/rtl.h
+++ b/gcc/rtl.h
@@ -3712,7 +3712,6 @@ extern unsigned hash_rtx_cb (const_rtx, machine_mode, int *, int *,
extern rtx regno_use_in (unsigned int, rtx);
extern int auto_inc_p (const_rtx);
extern bool in_insn_list_p (const rtx_insn_list *, const rtx_insn *);
-extern void remove_node_from_expr_list (const_rtx, rtx_expr_list **);
extern void remove_node_from_insn_list (const rtx_insn *, rtx_insn_list **);
extern int loc_mentioned_in_p (rtx *, const_rtx);
extern rtx_insn *find_first_parameter_load (rtx_insn *, rtx_insn *);
diff --git a/gcc/rtlanal.cc b/gcc/rtlanal.cc
index d78cc6024..ec95ecd6c 100644
--- a/gcc/rtlanal.cc
+++ b/gcc/rtlanal.cc
@@ -2878,35 +2878,6 @@ in_insn_list_p (const rtx_insn_list *listp, const rtx_insn *node)
return false;
}
-/* Search LISTP (an EXPR_LIST) for an entry whose first operand is NODE and
- remove that entry from the list if it is found.
-
- A simple equality test is used to determine if NODE matches. */
-
-void
-remove_node_from_expr_list (const_rtx node, rtx_expr_list **listp)
-{
- rtx_expr_list *temp = *listp;
- rtx_expr_list *prev = NULL;
-
- while (temp)
- {
- if (node == temp->element ())
- {
- /* Splice the node out of the list. */
- if (prev)
- XEXP (prev, 1) = temp->next ();
- else
- *listp = temp->next ();
-
- return;
- }
-
- prev = temp;
- temp = temp->next ();
- }
-}
-
/* Search LISTP (an INSN_LIST) for an entry whose first operand is NODE and
remove that entry from the list if it is found.
--
2.35.1
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 2/2] Avoid registering __builtin_setjmp_receiver label twice [PR101347]
2022-07-19 15:15 [PATCH 1/2] Remove unused remove_node_from_expr_list Alexander Monakov
@ 2022-07-19 15:15 ` Alexander Monakov
2022-07-20 7:39 ` Richard Biener
2022-07-20 7:34 ` [PATCH 1/2] Remove unused remove_node_from_expr_list Richard Biener
1 sibling, 1 reply; 6+ messages in thread
From: Alexander Monakov @ 2022-07-19 15:15 UTC (permalink / raw)
To: gcc-patches; +Cc: Alexander Monakov
The testcase in the PR demonstrates how it is possible for one
__builtin_setjmp_receiver label to appear in
nonlocal_goto_handler_labels list twice (after the block with
__builtin_setjmp_setup referring to it was duplicated).
remove_node_from_insn_list did not account for this possibility and
removed only the first copy from the list. Add an assert verifying that
duplicates are not present.
To avoid adding a label to the list twice, move registration of the
label from __builtin_setjmp_setup handling to __builtin_setjmp_receiver.
gcc/ChangeLog:
PR rtl-optimization/101347
* builtins.cc (expand_builtin) [BUILT_IN_SETJMP_SETUP]: Move
population of nonlocal_goto_handler_labels from here ...
(expand_builtin) [BUILT_IN_SETJMP_RECEIVER]: ... to here.
* rtlanal.cc (remove_node_from_insn_list): Verify that a
duplicate is not present in the remainder of the list.
---
gcc/builtins.cc | 15 +++++++--------
gcc/rtlanal.cc | 1 +
2 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/gcc/builtins.cc b/gcc/builtins.cc
index e6816d5c8..12a688dd8 100644
--- a/gcc/builtins.cc
+++ b/gcc/builtins.cc
@@ -7467,15 +7467,7 @@ expand_builtin (tree exp, rtx target, rtx subtarget, machine_mode mode,
tree label = TREE_OPERAND (CALL_EXPR_ARG (exp, 1), 0);
rtx_insn *label_r = label_rtx (label);
- /* This is copied from the handling of non-local gotos. */
expand_builtin_setjmp_setup (buf_addr, label_r);
- nonlocal_goto_handler_labels
- = gen_rtx_INSN_LIST (VOIDmode, label_r,
- nonlocal_goto_handler_labels);
- /* ??? Do not let expand_label treat us as such since we would
- not want to be both on the list of non-local labels and on
- the list of forced labels. */
- FORCED_LABEL (label) = 0;
return const0_rtx;
}
break;
@@ -7488,6 +7480,13 @@ expand_builtin (tree exp, rtx target, rtx subtarget, machine_mode mode,
rtx_insn *label_r = label_rtx (label);
expand_builtin_setjmp_receiver (label_r);
+ nonlocal_goto_handler_labels
+ = gen_rtx_INSN_LIST (VOIDmode, label_r,
+ nonlocal_goto_handler_labels);
+ /* ??? Do not let expand_label treat us as such since we would
+ not want to be both on the list of non-local labels and on
+ the list of forced labels. */
+ FORCED_LABEL (label) = 0;
return const0_rtx;
}
break;
diff --git a/gcc/rtlanal.cc b/gcc/rtlanal.cc
index ec95ecd6c..56da7435a 100644
--- a/gcc/rtlanal.cc
+++ b/gcc/rtlanal.cc
@@ -2899,6 +2899,7 @@ remove_node_from_insn_list (const rtx_insn *node, rtx_insn_list **listp)
else
*listp = temp->next ();
+ gcc_checking_assert (!in_insn_list_p (temp->next (), node));
return;
}
--
2.35.1
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] Remove unused remove_node_from_expr_list
2022-07-19 15:15 [PATCH 1/2] Remove unused remove_node_from_expr_list Alexander Monakov
2022-07-19 15:15 ` [PATCH 2/2] Avoid registering __builtin_setjmp_receiver label twice [PR101347] Alexander Monakov
@ 2022-07-20 7:34 ` Richard Biener
1 sibling, 0 replies; 6+ messages in thread
From: Richard Biener @ 2022-07-20 7:34 UTC (permalink / raw)
To: Alexander Monakov; +Cc: GCC Patches
On Tue, Jul 19, 2022 at 5:16 PM Alexander Monakov via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
> This function remains unused since remove_node_from_insn_list was cloned
> from it.
OK.
> gcc/ChangeLog:
>
> * rtl.h (remove_node_from_expr_list): Remove declaration.
> * rtlanal.cc (remove_node_from_expr_list): Remove (no uses).
> ---
> gcc/rtl.h | 1 -
> gcc/rtlanal.cc | 29 -----------------------------
> 2 files changed, 30 deletions(-)
>
> diff --git a/gcc/rtl.h b/gcc/rtl.h
> index 488016bb4..645c009a3 100644
> --- a/gcc/rtl.h
> +++ b/gcc/rtl.h
> @@ -3712,7 +3712,6 @@ extern unsigned hash_rtx_cb (const_rtx, machine_mode, int *, int *,
> extern rtx regno_use_in (unsigned int, rtx);
> extern int auto_inc_p (const_rtx);
> extern bool in_insn_list_p (const rtx_insn_list *, const rtx_insn *);
> -extern void remove_node_from_expr_list (const_rtx, rtx_expr_list **);
> extern void remove_node_from_insn_list (const rtx_insn *, rtx_insn_list **);
> extern int loc_mentioned_in_p (rtx *, const_rtx);
> extern rtx_insn *find_first_parameter_load (rtx_insn *, rtx_insn *);
> diff --git a/gcc/rtlanal.cc b/gcc/rtlanal.cc
> index d78cc6024..ec95ecd6c 100644
> --- a/gcc/rtlanal.cc
> +++ b/gcc/rtlanal.cc
> @@ -2878,35 +2878,6 @@ in_insn_list_p (const rtx_insn_list *listp, const rtx_insn *node)
> return false;
> }
>
> -/* Search LISTP (an EXPR_LIST) for an entry whose first operand is NODE and
> - remove that entry from the list if it is found.
> -
> - A simple equality test is used to determine if NODE matches. */
> -
> -void
> -remove_node_from_expr_list (const_rtx node, rtx_expr_list **listp)
> -{
> - rtx_expr_list *temp = *listp;
> - rtx_expr_list *prev = NULL;
> -
> - while (temp)
> - {
> - if (node == temp->element ())
> - {
> - /* Splice the node out of the list. */
> - if (prev)
> - XEXP (prev, 1) = temp->next ();
> - else
> - *listp = temp->next ();
> -
> - return;
> - }
> -
> - prev = temp;
> - temp = temp->next ();
> - }
> -}
> -
> /* Search LISTP (an INSN_LIST) for an entry whose first operand is NODE and
> remove that entry from the list if it is found.
>
> --
> 2.35.1
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] Avoid registering __builtin_setjmp_receiver label twice [PR101347]
2022-07-19 15:15 ` [PATCH 2/2] Avoid registering __builtin_setjmp_receiver label twice [PR101347] Alexander Monakov
@ 2022-07-20 7:39 ` Richard Biener
2022-07-20 8:31 ` Eric Botcazou
0 siblings, 1 reply; 6+ messages in thread
From: Richard Biener @ 2022-07-20 7:39 UTC (permalink / raw)
To: Alexander Monakov; +Cc: GCC Patches, Eric Botcazou
On Tue, Jul 19, 2022 at 5:17 PM Alexander Monakov via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
> The testcase in the PR demonstrates how it is possible for one
> __builtin_setjmp_receiver label to appear in
> nonlocal_goto_handler_labels list twice (after the block with
> __builtin_setjmp_setup referring to it was duplicated).
>
> remove_node_from_insn_list did not account for this possibility and
> removed only the first copy from the list. Add an assert verifying that
> duplicates are not present.
>
> To avoid adding a label to the list twice, move registration of the
> label from __builtin_setjmp_setup handling to __builtin_setjmp_receiver.
Eric is probably most familiar with this, but can you make sure to bootstrap
and test this on a SJLJ EH target? I'm not sure --enable-sjlj-exceptions is
well tested anywhere but on targets not supporting DWARF EH and the
configury is a bit odd suggesting the option is mostly ignored ...
Richard.
> gcc/ChangeLog:
>
> PR rtl-optimization/101347
> * builtins.cc (expand_builtin) [BUILT_IN_SETJMP_SETUP]: Move
> population of nonlocal_goto_handler_labels from here ...
> (expand_builtin) [BUILT_IN_SETJMP_RECEIVER]: ... to here.
> * rtlanal.cc (remove_node_from_insn_list): Verify that a
> duplicate is not present in the remainder of the list.
> ---
> gcc/builtins.cc | 15 +++++++--------
> gcc/rtlanal.cc | 1 +
> 2 files changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/gcc/builtins.cc b/gcc/builtins.cc
> index e6816d5c8..12a688dd8 100644
> --- a/gcc/builtins.cc
> +++ b/gcc/builtins.cc
> @@ -7467,15 +7467,7 @@ expand_builtin (tree exp, rtx target, rtx subtarget, machine_mode mode,
> tree label = TREE_OPERAND (CALL_EXPR_ARG (exp, 1), 0);
> rtx_insn *label_r = label_rtx (label);
>
> - /* This is copied from the handling of non-local gotos. */
> expand_builtin_setjmp_setup (buf_addr, label_r);
> - nonlocal_goto_handler_labels
> - = gen_rtx_INSN_LIST (VOIDmode, label_r,
> - nonlocal_goto_handler_labels);
> - /* ??? Do not let expand_label treat us as such since we would
> - not want to be both on the list of non-local labels and on
> - the list of forced labels. */
> - FORCED_LABEL (label) = 0;
> return const0_rtx;
> }
> break;
> @@ -7488,6 +7480,13 @@ expand_builtin (tree exp, rtx target, rtx subtarget, machine_mode mode,
> rtx_insn *label_r = label_rtx (label);
>
> expand_builtin_setjmp_receiver (label_r);
> + nonlocal_goto_handler_labels
> + = gen_rtx_INSN_LIST (VOIDmode, label_r,
> + nonlocal_goto_handler_labels);
> + /* ??? Do not let expand_label treat us as such since we would
> + not want to be both on the list of non-local labels and on
> + the list of forced labels. */
> + FORCED_LABEL (label) = 0;
> return const0_rtx;
> }
> break;
> diff --git a/gcc/rtlanal.cc b/gcc/rtlanal.cc
> index ec95ecd6c..56da7435a 100644
> --- a/gcc/rtlanal.cc
> +++ b/gcc/rtlanal.cc
> @@ -2899,6 +2899,7 @@ remove_node_from_insn_list (const rtx_insn *node, rtx_insn_list **listp)
> else
> *listp = temp->next ();
>
> + gcc_checking_assert (!in_insn_list_p (temp->next (), node));
> return;
> }
>
> --
> 2.35.1
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] Avoid registering __builtin_setjmp_receiver label twice [PR101347]
2022-07-20 7:39 ` Richard Biener
@ 2022-07-20 8:31 ` Eric Botcazou
2022-07-20 13:13 ` Alexander Monakov
0 siblings, 1 reply; 6+ messages in thread
From: Eric Botcazou @ 2022-07-20 8:31 UTC (permalink / raw)
To: Richard Biener; +Cc: Alexander Monakov, gcc-patches, GCC Patches
> Eric is probably most familiar with this, but can you make sure to bootstrap
> and test this on a SJLJ EH target? I'm not sure --enable-sjlj-exceptions
> is well tested anywhere but on targets not supporting DWARF EH and the
> configury is a bit odd suggesting the option is mostly ignored ...
This is a specific circuitry for __builtln_setjmp so it is *not* exercised by
the SJLJ exception scheme. It used to be exercised by the GNAT bootstrap, but
that's no longer the case either.
I think that the fix is sensible, assuming that it passes the C testsuite.
--
Eric Botcazou
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] Avoid registering __builtin_setjmp_receiver label twice [PR101347]
2022-07-20 8:31 ` Eric Botcazou
@ 2022-07-20 13:13 ` Alexander Monakov
0 siblings, 0 replies; 6+ messages in thread
From: Alexander Monakov @ 2022-07-20 13:13 UTC (permalink / raw)
To: Eric Botcazou; +Cc: Richard Biener, GCC Patches
On Wed, 20 Jul 2022, Eric Botcazou wrote:
> > Eric is probably most familiar with this, but can you make sure to bootstrap
> > and test this on a SJLJ EH target? I'm not sure --enable-sjlj-exceptions
> > is well tested anywhere but on targets not supporting DWARF EH and the
> > configury is a bit odd suggesting the option is mostly ignored ...
>
> This is a specific circuitry for __builtln_setjmp so it is *not* exercised by
> the SJLJ exception scheme. It used to be exercised by the GNAT bootstrap, but
> that's no longer the case either.
>
> I think that the fix is sensible, assuming that it passes the C testsuite.
Yes, it passes the usual regtest. Thanks, applying to trunk.
Alexander
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-07-20 13:13 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-07-19 15:15 [PATCH 1/2] Remove unused remove_node_from_expr_list Alexander Monakov
2022-07-19 15:15 ` [PATCH 2/2] Avoid registering __builtin_setjmp_receiver label twice [PR101347] Alexander Monakov
2022-07-20 7:39 ` Richard Biener
2022-07-20 8:31 ` Eric Botcazou
2022-07-20 13:13 ` Alexander Monakov
2022-07-20 7:34 ` [PATCH 1/2] Remove unused remove_node_from_expr_list Richard Biener
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).